-
Notifications
You must be signed in to change notification settings - Fork 71
Automatically bootstrap using environment variables, if present. #811
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
lib/cog/bootstrap.ex
Outdated
| true | ||
| end | ||
|
|
||
| false |
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.
Why this return value? And the true above in the unless block?
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 ... don't remember. Looking at the code now it doesn't seem to be used. I think I just wanted to indicate whether a bootstrap actually happened or not as a result of that call. I can easily take taht out if we want.
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.
Yeah let's take that out. Feels weird that it would always return false.
|
Have you tested to see if the transactions behave as expected? If you're not explicitly checking return values and rolling back the transaction on your own, you need to be using |
|
@christophermaier I made the functions you mentioned private and left a comment above about he return value from the As far as the transaction goes, yes, I tested it. If you look in |
|
Ah, yeah, I missed the I'd rather remove those booleans... it's potentially confusing. |
d2e7509 to
3dedfa1
Compare
|
Added logging for missing variables. I only emit this if at least one of the Also removed the booleans. |
|
👍 |
3dedfa1 to
7b68f13
Compare
|
Thanks, @christophermaier for the review. You made my code better. I'm going to go ahead and merge as soon as CI finishes since I've addressed all the feedback. Here's where we landed: Victory!Environment: All variables present: Validation: Error Logging:Cog variables present, but no relay variables: Some, but not all, Cog variables present: |
Create an administrative user using values from the environment, if present. If any of the following variables are not set, the bootstrap will not be performed. Required variables:
Additionally, if the user bootstrap completes successfully and the
RELAY_IDandRELAY_COG_TOKENvariables are set, a relay will be created using the variables and assigned to a default relay group, which will also be created. If the relevant variables are not present, this step will be skipped.If any creation attempt fails, the entire bootstrap process will be rolled back.