-
Notifications
You must be signed in to change notification settings - Fork 1
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
[ENH] Update environment variables and remove deprecated version
tag
#65
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.
Thanks @alyssadai, looks good.
The changes that relate to auth need a clarification that auth is under active development and unless you are deploying nightly/developing yourself you can ignore them for now.
We don't want folks who are making deployments now (as e.g. workshop attendees will probably do) to get confused.
I'll still approve it, but please take a look at these comments first and then 🧑🍳
`NB_ENABLE_AUTH` Yes Whether to enable authentication for cohort queries. One of [true, false] `true` Docker, Python | ||
`NB_QUERY_CLIENT_ID` Yes OAuth client ID for the query tool. Required if NB_ENABLE_AUTH is set to true. - Docker, Python |
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.
Like below, I think we should clarify here that these variables are not (yet) needed for prod deployments because auth is under active development still. Not sure how to best do that. Maybe you have a good idea?
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.
How about in parentheses at the start of the description (Experimental, for dev deploments only)
/ (PREVIEW)
? Or something similar
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.
Will use (Experimental, for dev deployments only) for now :)
version
tagversion
tag
🚀 PR was released in |
version
tag in docker-compose.yml #56NB_GRAPH_ROOT_HOME
from template.env #62NB_FEDERATE_REMOTE_PUBLIC_NODES
to docker-compose.yml #64Changes proposed in this pull request:
Checklist
This section is for the PR reviewer
[ENH]
,[FIX]
,[REF]
,[TST]
,[CI]
,[MNT]
,[INF]
,[MODEL]
,[DOC]
) (see our Contributing Guidelines for more info)Closes #XXXX
For new features:
For bug fixes: