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

Non-robust default value for NB_API_QUERY_URL in Compose recipes #21

Closed
alyssadai opened this issue Feb 27, 2024 · 3 comments · Fixed by #42
Closed

Non-robust default value for NB_API_QUERY_URL in Compose recipes #21

alyssadai opened this issue Feb 27, 2024 · 3 comments · Fixed by #42
Assignees
Labels
released This issue/pull request has been released.

Comments

@alyssadai
Copy link
Contributor

alyssadai commented Feb 27, 2024

As (re-)discovered in neurobagel/planning#111 (comment), when the query tool is part of a deployment that is meant to be accessed from other machines, the URL used for the API that it talks to has to be provided as it would appear to an external user on their own machine.

The query tool essentially always behaves as if the user has downloaded it and then run it on their own machine. So, NB_API_QUERY_URL must be set to a URL that makes sense from outside the machine hosting the API, such as the public IP of the host, or the subdomain for the API if NGINX is set up on the machine.

One exception is if a user tries deploying our API stack on their local machine (and then tries the query tool from the same machine), in which case NB_API_QUERY_URL would need to be localhost (also because their laptop would not have a unique public IP). This should be documented clearly in a note to the user.

Meanwhile, we should update the default in the .env file to a clearly toy/must-be-changed URL, e.g.:

@alyssadai alyssadai added the flag:schedule Flag issue that should go on the roadmap or backlog. label Feb 28, 2024
@surchs surchs moved this to Backlog in Neurobagel Feb 28, 2024
@surchs surchs removed the flag:schedule Flag issue that should go on the roadmap or backlog. label Feb 28, 2024
@alyssadai alyssadai changed the title Non-robust default value for API_QUERY_URL in federation recipe Non-robust default value for API_QUERY_URL in Compose recipes Mar 13, 2024
@rmanaem rmanaem changed the title Non-robust default value for API_QUERY_URL in Compose recipes Non-robust default value for NB_API_QUERY_URL in Compose recipes Apr 3, 2024
@rmanaem rmanaem added the flag:discuss Flag issue that needs to be discussed before it can be implemented. label Apr 3, 2024
@surchs surchs moved this from Backlog to Specify - Active in Neurobagel Apr 10, 2024
@surchs
Copy link
Contributor

surchs commented Apr 10, 2024

Starting the spec here: @neurobagel/dev between these two options:

12.34.56.78
XX.XX.XX.XX

I prefer the XXX one because it is clearly a placeholder, whereas the 12.34.etc one requires an extra glance to understand as a placeholder. With an extra comment to explain: please change this and use localhost if run only locally, I think this should be fine.

Once we get to #30, we could make this even more visible by putting the NB_API_QUERY_URL in the section of ENV variables to pay attention to, i.e. at the top of the file.

@surchs surchs moved this from Specify - Active to Specify - Done in Neurobagel Apr 16, 2024
@alyssadai
Copy link
Contributor Author

I think the XXX option is fine!

@alyssadai alyssadai moved this from Specify - Done to Implement - Active in Neurobagel Apr 16, 2024
@alyssadai alyssadai removed the flag:discuss Flag issue that needs to be discussed before it can be implemented. label Apr 16, 2024
@alyssadai alyssadai self-assigned this Apr 16, 2024
@surchs surchs assigned surchs and unassigned surchs Apr 16, 2024
@alyssadai alyssadai moved this from Implement - Active to Implement - Done in Neurobagel Apr 16, 2024
@surchs surchs moved this from Implement - Done to Review - Active in Neurobagel Apr 16, 2024
@github-project-automation github-project-automation bot moved this from Review - Active to Review - Done in Neurobagel Apr 18, 2024
Copy link
Contributor

neurobagel-bot bot commented Aug 7, 2024

🚀 Issue was released in v0.0.1 🚀

@neurobagel-bot neurobagel-bot bot added the released This issue/pull request has been released. label Aug 7, 2024
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.
Projects
Archived in project
3 participants