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

Add and document env vars for API root paths #102

Closed
3 tasks done
alyssadai opened this issue Jan 8, 2025 · 5 comments · Fixed by #106
Closed
3 tasks done

Add and document env vars for API root paths #102

alyssadai opened this issue Jan 8, 2025 · 5 comments · Fixed by #106
Assignees
Labels
released This issue/pull request has been released. usability Issue affecting user or developer experience.

Comments

@alyssadai
Copy link
Contributor

alyssadai commented Jan 8, 2025

  • NB_NAPI_ROOT_PATH
  • NB_FAPI_ROOT_PATH
  • Specify that paths should not have a trailing slash

Related to:

@alyssadai alyssadai added flag:schedule Flag issue that should go on the roadmap or backlog. usability Issue affecting user or developer experience. labels Jan 8, 2025
@surchs surchs moved this to Backlog in Neurobagel Jan 9, 2025
@surchs surchs removed the flag:schedule Flag issue that should go on the roadmap or backlog. label Jan 9, 2025
@alyssadai alyssadai moved this from Backlog to Specify - Active in Neurobagel Jan 23, 2025
@alyssadai alyssadai moved this from Specify - Active to Specify - Done in Neurobagel Jan 23, 2025
@alyssadai alyssadai moved this from Specify - Done to Implement - Active in Neurobagel Jan 30, 2025
@alyssadai alyssadai self-assigned this Jan 31, 2025
@alyssadai
Copy link
Contributor Author

@neurobagel/dev I realized we have a slight name discrepancy between the root/base path in the query tool (NB_QUERY_APP_BASE_PATH), and the ones in the APIs. What are your preferences in terms of whether we should name these *_ROOT_PATH or *_BASE_PATH?

@alyssadai alyssadai moved this from Implement - Active to Implement - Done in Neurobagel Feb 1, 2025
@surchs
Copy link
Contributor

surchs commented Feb 3, 2025

I think _BASE_PATH is the more usual name here (e.g. https://swagger.io/docs/specification/v3_0/api-host-and-base-path/). So I'd prefer _BASE_PATH

@rmanaem
Copy link
Contributor

rmanaem commented Feb 3, 2025

I've seen *BASE* used more often in documentations. We could differentiate between the env var in API and query tool using *_BASE_PATH and *_BASE_URL.

@alyssadai
Copy link
Contributor Author

alyssadai commented Feb 4, 2025

Okay, _BASE_PATH sounds good!

I'd prefer to keep the same name across tools, since for both the query tool and the API only the path prefix needs to be specified, not the full url - so *_BASE_URL may be confusing.

Moving this into tracked until the variable name change is implemented in the APIs (will fix as part of PRs to standardize trailing slashes).

@alyssadai alyssadai moved this from Implement - Done to Implement - Track in Neurobagel Feb 4, 2025
@surchs surchs moved this from Implement - Track to Review - Active in Neurobagel Feb 4, 2025
@surchs surchs moved this from Review - Active to Implement - Track in Neurobagel Feb 4, 2025
@alyssadai alyssadai moved this from Implement - Track to Implement - Done in Neurobagel Feb 6, 2025
@alyssadai alyssadai moved this from Implement - Done to Implement - Active in Neurobagel Feb 6, 2025
@alyssadai alyssadai moved this from Implement - Active to Implement - Track in Neurobagel Feb 6, 2025
@alyssadai alyssadai moved this from Implement - Track to Implement - Active in Neurobagel Feb 12, 2025
@alyssadai alyssadai moved this from Implement - Active to Review - Active in Neurobagel Feb 12, 2025
@github-project-automation github-project-automation bot moved this from Review - Active to Review - Done in Neurobagel Feb 14, 2025
Copy link
Contributor

🚀 Issue 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released This issue/pull request has been released. usability Issue affecting user or developer experience.
Projects
Status: Review - Done
3 participants