-
Notifications
You must be signed in to change notification settings - Fork 20
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
Add a 'Connections' property #110
Conversation
Thanks for the PR! Let us know where you're getting stuck testing and we'll get you sorted out :) |
Have you had a chance to test this? |
I apologize for the very long delay on this. I wasn't originally intending to build or test, as I had no experience with any of this... including Rust, Docker, Make, Typescript, and on and on. I only created the draft PR after being requested to. Anyway, I finally got the motivation to learn and was able to spin up an Ubuntu server on a VM to build the .s9pk and then load it onto a StartOS VM. The new property worked as expected! I've attached a screenshot below showing the property value in the Web UI compared to running the The only change I had to make was to the root project's Makefile (unrelated to this PR):
|
This looks awesome, is it ready for review? Happy to test |
Marked as ready for review. This is my first PR (ever), so feel free to change anything or let me know what I should change or do differently in the future. |
Thanks for trying this out and sorry that there was an issue. I've merged the latest from the Start9Lab:master to my property-test branch, rebuilt with Btw, the 0.0GiB disk usage is because I sideloaded the .s9pk package file I created onto a VM with a freshly installed StartOS. I did not update a Bitcoin Core service on an existing StartOS machine. So it was still syncing headers in the VM when I took that screenshot. I forced the inbound connection by adding the VM node's peer address to my main bitcoin node. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested good for me - I'll let @dr-bonez take a look for merge
install: | ||
ifeq (,$(wildcard ~/.embassy/config.yaml)) | ||
@echo; echo "You must define \"host: http://server-name.local\" in ~/.embassy/config.yaml config file first"; echo | ||
install: $(PKG_ID).s9pk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is wrong - it will force universal package build on each install making arm and x86 targets useless
ifeq (,$(wildcard ~/.embassy/config.yaml)) | ||
@echo; echo "You must define \"host: http://server-name.local\" in ~/.embassy/config.yaml config file first"; echo | ||
install: $(PKG_ID).s9pk | ||
ifeq (,$(wildcard ./start9/config.yaml)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are searching for the start-sdk config file that resides (by default) in users home directory and has host defined for easy installation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dr-bonez take a look at it please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed main.rs
looks good to my eye. Good catch on the Makefile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't built and tested, but the code looks good.
Merging and manually fixing the Makefile issue as @biotic21 has become unresponsive. PR inbound |
Thank you all for the help getting this completed! Sorry that I didn't realize you were waiting for me to respond to something. I'm not sure I'm knowledgeable enough about the ins and outs of the Makefile anyway to be of much help. But thanks again! |
No worries, we dropped the ball for a while as well. Thank you very much for the PR |
Add a new property to display the number of peers connected, including inbound and outbound.
Code not tested, so creating a draft PR as requested here.