search

conda-incubator / conda-store

Data science environments, for collaboration. ✨
https://conda.store
BSD 3-Clause "New" or "Revised" License
150 stars 50 forks source link

Ensure api url setting has trailing slash #997

Closed soapy1 closed 3 days ago

soapy1 commented 3 days ago

Fixes https://github.com/conda-incubator/conda-store/issues/995

Description

This is required for the "log and artifact" links in the ui to work.

For example, follow the instructions in #995 to reproduce the issue. With this change, the links provided in the Logs and Artifacts section correctly point to /conda-store/api/v1/<rest of the api path>.

Pull request checklist

netlify[bot] commented 3 days ago

Deploy Preview for conda-store canceled.

Name Link
Latest commit f95db459747857c1c740a13eb99b68f0f964a9f8
Latest deploy log https://app.netlify.com/sites/conda-store/deploys/673fb7a23ca5780008995272
peytondmurray commented 3 days ago

Looks like this is not being tested, otherwise we would have caught this. Would it be possible to write a test to hit these logs/artifacts endpoints with and without trailing slashes?

soapy1 commented 3 days ago

Would it be possible to write a test to hit these logs/artifacts endpoints with and without trailing slashes?

Thinking about this issue more, I think it would be better for this issue to be fixed in conda-store-ui opposed to conda-store.

The crux of the issue is that the logs/artifacts in the ui are links to some api endpoint. Without this slash the url_prefix is ignored for building these links. But, the rest of the api interactions between conda-store and conda-store-ui work without this trailing slash. So, that makes me think that there is an issue with how the ui is generating the links.

I took a quick look at the ui code, but I don't really understand how it works so I could be really off base here. @peytondmurray @gabalafou if you have some time could you take a look?

soapy1 commented 3 days ago

ok so, I think this fixes things on the ui side https://github.com/conda-incubator/conda-store-ui/pull/442 and this PR can be closed.

gabalafou commented 3 days ago

I think that the front-end app should be robust enough to handle whether an implementer supplies the base URL with or without the trailing slash

Hope you don't mind me closing this pull request in favor of https://github.com/conda-incubator/conda-store-ui/pull/443.