-
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] Added NB_QUERY_URL_PATH
#69
Conversation
@rmanaem, could you take a look at the merge conflicts? |
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.
Two 🍒, otherwise 🧑🍳!
For the sake of coordinating with the changes to the query tool, should we wait to merge this until neurobagel/query-tool#227 is ready to be merged?
@@ -76,6 +76,7 @@ services: | |||
environment: | |||
NB_API_QUERY_URL: ${NB_API_QUERY_URL} |
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.
A thought: I'm realizing that the name NB_API_QUERY_URL
is pretty confusing and ambiguous in terms of whether it points to the query tool or API, now that we also have NB_QUERY_URL_PATH
.
We should probably consider renaming NB_API_QUERY_URL
to sth clearer, either as part of #30 or the issue concerning query tool -> f-API or query tool -> n-API. Maybe you can link this PR there, or vice versa, once the issue is open? :)
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 think it's better to be addressed in #30 but we can also discuss it in neurobagel/query-tool#225
Co-authored-by: Alyssa Dai <[email protected]>
Co-authored-by: Alyssa Dai <[email protected]>
🚀 PR was released in |
NB_QUERY_URL_PATH
env var #68Checklist
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: