-
Notifications
You must be signed in to change notification settings - Fork 2
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 beacon network #259
Add beacon network #259
Conversation
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.
minor: a documentation request; otherwise lgtm
docs/installation.md
Outdated
@@ -158,6 +158,14 @@ If using Beacon, first copy the configuration file: | |||
Then update any config values as needed at `lib/beacon/config/beacon_config.json` | |||
and `lib/beacon/config/beacon_cohort.json`. | |||
|
|||
If using the beacon network, copy the configuration file: |
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.
capital B for beacon for consistency
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.
(╯°□°)╯︵ ┻━┻
``` | ||
|
||
and update values at `lib/beacon/config/beacon_network_config.json`. | ||
|
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.
Can a note about this be added to the v17 migration guide? Similar to here, basically
docs/migrating_to_17.md
Outdated
@@ -84,6 +84,21 @@ bento_authz public-data-access counts | |||
``` | |||
|
|||
|
|||
## 5. Optionally, add Beacon network | |||
|
|||
To host a network of beacons, with a corresponding UI in bento_public, first copy the config file: |
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.
either Bento Public or bento_public
with code ticks around it
docs/migrating_to_17.md
Outdated
```bash | ||
BENTO_BEACON_NETWORK_ENABLED='true' | ||
``` | ||
|
||
## 5. Start Bento |
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.
make this 6
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.
They're so closely tied I didn't see the point separating them, especially as optional steps.
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.
even still, I don't think it makes sense to have two step 5s, with one of them being optional and the other being required (and coming after the first step 5)
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.
right, I was thinking of the env step as optional also, but I guess should be thrown in regardless
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.
lgtm
Add handling for beacon network. Handled as extra beacon configuration, no new containers. Accompanies beacon pr #93., bento_public pr #165.
TODO: