Fix incorrect fields in info endpoint #1046
Conversation
jjnesbitt
added
patch
| def test_rest_info(api_client): | ||
| resp = api_client.get('/api/info/') | ||
| assert resp.status_code == 200 | ||
| assert resp.json() == { | ||
| 'schema_version': settings.DANDI_SCHEMA_VERSION, | ||
| 'schema_url': schema_url, | ||
| 'version': versioneer.get_version(), | ||
| 'cli-minimal-version': '0.14.2', | ||
| 'cli-bad-versions': [], | ||
| 'services': { | ||
| 'api': {'url': settings.DANDI_API_URL}, | ||
| 'webui': {'url': settings.DANDI_WEB_APP_URL}, | ||
| 'jupyterhub': {'url': settings.DANDI_JUPYTERHUB_URL}, | ||
| }, | ||
| } |
There was a problem hiding this comment.
Since this endpoint is something that the CLI depends on, I think having a similar test as part of the CLI integration tests would be valuable (thoughts @jwodder @yarikoptic?)
There was a problem hiding this comment.
There effectively is such a test already: https://github.com/dandi/dandi-cli/blob/00ee4ddf8785a91ff5bd893497b018ef5fce6437/dandi/tests/test_utils.py#L303
There was a problem hiding this comment.
That's a really good point. I think adding a CLI Integration test is important, since that's what would break if we were to change this. We probably want to merge this as soon as possible, but that test could be added after the fact.
There was a problem hiding this comment.
@jwodder I see a couple of problems with that test
- We're not specifying the
DANDI_REDIRECTOR_BASEenvironment variable currently, so this test is either skipped or tested against the currently deployed prod (not totally sure of the behavior from looking at that file). In either case, the current change isn't tested. - Even if we did specify that env variable, this redirection doesn't happen locally, it only happens in Netlify, so the test would fail in that way.
We should probably at least add an almost identical test aimed at the locally spun up API instance, instead of prod.
|
CLI Integration tests are failing, but they seem to be due to a prior change, not this one (evidenced by this same failure occurring prior to this PR here). It seems the integration tests need some additional work on top of this, per our conversation on this PR. I'm going to merge this, since this is currently causing issue in prod, and these changes can be applied after the fact. |
|
🚀 PR was released in |
Closes #1044
The test is pretty barebones but will ensure tests fail if the endpoint structure is modified.
This also fixes a typo in the
DJANGO_DANDI_JUPYTERHUB_URLenv var indev/.env.docker-composeanddev/.env.docker-compose-native.