Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Plugin Architecture #670
Plugin Architecture #670
Changes from 31 commits
7b43e43
c155135
927cc82
1fad8db
1d9fb95
08ad2bd
42a4118
7bc7ab6
5ba06aa
9071720
3f125d6
b904879
1789a53
77ac67c
fcd871d
7a4ff3c
f042913
e23a728
0196643
2bb6bbc
89f4d19
98c002d
7817cc7
dc05443
08b8c32
0e33b48
ec17d24
fd80b3b
3b575d6
33a9330
27ac1f3
39da86d
925f17a
e351787
7311bc2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Any reason to put this network here instead of
/test/data
? This location will make it a "default" network that gets copied into all new warnets. Maybe the idea is to have a plugin template as a default? Either way I prefer networks that are run by tests to live in/test/data
.Especially because this network.yaml generates tons of LN activity between two specific nodes, not like general network activity. It seems less default-y
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 like having the 'hello' network as as something that users get when they
init
a new warnet user directory because I think it's helpful to have an "all in" example for users to look at.That said, I can understand creating a copy of the hello network in the testing directory so that all the test stuff is isolated.
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.
Aren't we trying to avoid kubectl commands in the docs? Lets just reccomend
warnet logs -f <pod name>
?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.
warnet logs -f <pod name>
runs into an issue -- it can't determine the primary container.We could get around this by choosing to treat the first container as the primary container.
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.
there is some logic for this already. maybe we can address in a followup:
warnet/src/warnet/control.py
Lines 411 to 425 in 5039591
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.
If continue using the existing logic, we would need to dictate that plugins set a PLUGIN_CONTAINER as their primary container. I'm not totally opposed to that.