dandi / dandi-archive

DANDI API server and Web app
https://dandiarchive.org
13 stars 12 forks source link

Cannot publish on staging due to "invalid" URL #961

Closed jwodder closed 2 years ago

jwodder commented 2 years ago

Dandisets on staging currently fail validation with:

"version_validation_errors": [
    {
        "field": "url",
        "message": "'https://gui-staging.dandiarchive.org/dandiset/101514/0.220310.1340' does not match '^https://dandiarchive.org/dandiset/\\\\d{6}/\\\\d+\\\\.\\\\d+\\\\.\\\\d+$'"
    }
]
yarikoptic commented 2 years ago

ping on this @waxlamp @dchiquito et al

waxlamp commented 2 years ago

Thanks @yarikoptic; looking into it.

yarikoptic commented 2 years ago

@waxlamp any updates on this?

jjnesbitt commented 2 years ago

The root of this issue is in dandischema, as there we check that the version url is as expected. However, currently this only accounts for the main deployment (dandiarchive.org) and local dev (localhost), so the staging deployment is seen as invalid.

It seems to me the approach to fix this is to either add the staging url to that regex, or to change the behavior in dandischema to instead read the value of an environment variable (e.g. DANDI_ALLOWED_LOCALHOST_URLS) to determine which urls are valid.

I prefer the second approach.

yarikoptic commented 2 years ago

thanks @AlmightyYakob . I haven't analyzed why it started to happen, and followed your recommendation to propose https://github.com/dandi/dandischema/pull/128 . I guess let's continue on that particular possible fix there

edit: forgot to mention relevant somewhat https://github.com/dandi/dandischema/issues/67

jjnesbitt commented 2 years ago

I haven't analyzed why it started to happen

It probably started to occur as a result of https://github.com/dandi/dandi-archive/pull/946

yarikoptic commented 2 years ago

might be, might be ... thanks @AlmightyYakob for pointing that PR out since I was wondering. Indeed many moving parts and even though integration test of dandi-cli in that PR was green some particular change (dandischema? did not use that freshly build docker image) was the one which let regression to go unnoticed. Eh -- never enough tests! ;)

yarikoptic commented 2 years ago

FWIW, tried to see if problem is gone with an upgrade of dandischema on the server, but the error persists may be due to need to revalidate (#1080) and on a new dandiset I do not see that error, but see a different one (#1081) preventing dandiset from publishing.

jwodder commented 2 years ago

This was resolved some time ago.