Skip to content
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] Add variables for API root paths + query site header script, add default values #106

Merged
merged 11 commits into from
Feb 14, 2025

Conversation

alyssadai
Copy link
Contributor

@alyssadai alyssadai commented Feb 1, 2025

Changes proposed in this pull request:

  • Add new variables NB_NAPI_BASE_PATH, NB_FAPI_BASE_PATH, NB_QUERY_HEADER_SCRIPT
  • Added default values for all optional variables in docker-compose.yml

Housekeeping:

Checklist

This section is for the PR reviewer

  • PR has an interpretable title with a prefix ([ENH], [FIX], [REF], [TST], [CI], [MNT], [INF], [MODEL], [DOC]) (see our Contributing Guidelines for more info)
  • PR has a label for the release changelog or skip-release (to be applied by maintainers only)
  • PR links to GitHub issue with mention Closes #XXXX
  • Tests pass
  • Checks pass

For new features:

  • Tests have been added

For bug fixes:

  • There is at least one test that would fail under the original bug conditions.

@alyssadai alyssadai added the pr-minor Non-breaking feature or enhancement, will increment minor version (0.+1.0) label Feb 1, 2025
@surchs surchs self-requested a review February 4, 2025 19:13
@alyssadai alyssadai added the release Create a release when this PR is merged label Feb 11, 2025
@alyssadai
Copy link
Contributor Author

@surchs, this PR is also ready for review!

Copy link
Contributor

@surchs surchs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good. I left a couple of suggestions. I think the main thing is whether we rename the env variables on the tools to match the names we use in the recipe. I think that'd help quite a bit to avoid confusion.

🧑‍🍳

@alyssadai alyssadai merged commit db90c46 into main Feb 14, 2025
@alyssadai alyssadai deleted the add-new-env-vars branch February 14, 2025 17:04
Copy link
Contributor

🚀 PR was released in v0.5.0 🚀

@neurobagel-bot neurobagel-bot bot added the released This issue/pull request has been released. label Feb 14, 2025
@alyssadai alyssadai changed the title [ENH] Add environment variables for API root paths and query site header script [ENH] Add variables for API root paths + query site header script, add optional variable defaults Feb 14, 2025
@alyssadai alyssadai changed the title [ENH] Add variables for API root paths + query site header script, add optional variable defaults [ENH] Add variables for API root paths + query site header script, add default values everywhere Feb 14, 2025
@alyssadai alyssadai changed the title [ENH] Add variables for API root paths + query site header script, add default values everywhere [ENH] Add variables for API root paths + query site header script, add default values Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-minor Non-breaking feature or enhancement, will increment minor version (0.+1.0) release Create a release when this PR is merged released This issue/pull request has been released.
Projects
None yet
2 participants