-
-
Notifications
You must be signed in to change notification settings - Fork 805
Uncomplicate btcblock alias and fix 301 error #593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The btcblock alias was overly complicated requiring multiple cut commands and multiple line reversals due to using wget (which wasn't even installed on debian by default, whereas curl is). Using curl with the silent flag reduces complexity. Also removing the cat command double redirection using HERE doc notation and complicated escaping is accomplished with a single echo command and simply append redirect. Update the mainnet get block call with curl and fix the 301 error received by calling url in plain text.
@histamineblkr Thanks for the PR! @shannona is the lead for this section, but probably will not be able to review/test it for a few weeks as we are under a crunch. It also connects to other repos (such as bitcoin-standup) to sync with their scripts. But your PR seems reasonable! |
Looks very helpful, thank you! I'll need to spin up a Bitcoin instance to check it out, and as Christopher mentioned we're crunching at the moment, so hopeful able to pull this in before too long! |
Awesome! No worries on time line since it's completely up to you and the team. I will be working through the docs slowly on my off time so I will try to update things as I go. A couple separate things I noticed are the bitcoin standup script depends on I guess my question is, would it be helpful if I fixed this and created a different pull request? I think we can make the shell script a bit more robust and better at handling errors. Some simple best practices and error handling will go a long away. I would be happy to put something together if it would be useful and I'd use this best practices reference since it captures most things I'd do. |
Brandon, we would welcome your contribution, especially as a new release of Bitcoin will be out soon. Last year some major changes to how the Bitcoin binaries are verified put us backward on our roadmap. Instead of finishing fixes to a contribution that would refactor the script to allow for other bitcoin apps to be installed, we had to focus on just keeping the old script (and Gordian Server macOS app) running. You might want to peek at BlockchainCommons/Bitcoin-Standup-Scripts#15 — it needs some puzzling out to compleye, but would offer some nice integrations. |
That is a significant refactor going on. I definitely have some ideas on how this could go forward. In your
basically the 2 scripts (Linode and regular) do that reasonably well. Each script could be improved in their own right, but I think they mostly accomplish this. Before I do too much work or try to refactor/fix things, I should probably understand what the goal going forward is, if there is one? I will try to give how I view the current implementation: |
Thanks, @histamineblkr , tested it out and this looks great. |
Regarding the larger refactor of StandupScript: I'd classify it as waiting-to-be-assigned. The biggest problem is that one of our interns from 2020 did some great work, but we didn't have the resources to fully test it, and then emergency updates for changes in the Bitcoin releases forced us to make changes to the scripts in advance of that, and suddenly the PR was no longer cleanly mergeable. As for the two scripts: they're mostly parallel, but we're only putting the big additions (like new apps) into Standup, mainly because the interface for adding more services into the Linode script would get ridiculous (but I also think that Linode isn't quite the dominant force in the reasonably-priced cloud computing area that it was when we started using it 10 years ago, or so). So, if we had a fix or update I'd like to see them in both, but for making the Standup script more expansive, that's likely only going to be the shell script. My rough TODO at this point is:
So I'm going to create an issue for that first topic on the Standup repo, and you'd like to help with that, that'd be wonderful, and then let me know if you'd like to go on with the others (and/or if you have other ideas for improvement). |
I wanted to touch bases, @histamineblkr, to see if you were still interested in this. |
The btcblock alias was overly complicated requiring multiple cut commands and multiple line reversals due to using wget (which wasn't even installed on debian by default, whereas curl is). Using curl with the silent flag reduces complexity. Also removing the cat command double redirection using HERE doc notation and complicated escaping is accomplished with a single echo command and simply append redirect.
Update the mainnet get block call with curl and fix the 301 error received by calling url in plain text.