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

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

Closed
jwodder opened this issue Mar 10, 2022 · 9 comments
Closed

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

jwodder opened this issue Mar 10, 2022 · 9 comments

Comments

@jwodder
Copy link
Member

jwodder commented Mar 10, 2022

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
Copy link
Member

ping on this @waxlamp @dchiquito et al

@waxlamp
Copy link
Member

waxlamp commented Mar 17, 2022

Thanks @yarikoptic; looking into it.

@yarikoptic
Copy link
Member

@waxlamp any updates on this?

@jjnesbitt
Copy link
Member

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 added a commit to dandi/dandi-schema that referenced this issue Apr 1, 2022
… needed

Ref: dandi/dandi-archive#961

I have decided to not hardcode gui-staging. one because the fix is targetting
specifically dandisets testing, and otherwise AFAIK is not needed elsewhere.
If something wants to upload to staging, they can also adjust their CI to point
to whatever instances they upload to
@yarikoptic
Copy link
Member

yarikoptic commented Apr 1, 2022

thanks @AlmightyYakob . I haven't analyzed why it started to happen, and followed your recommendation to propose dandi/dandi-schema#128 . I guess let's continue on that particular possible fix there

edit: forgot to mention relevant somewhat dandi/dandi-schema#67

@jjnesbitt
Copy link
Member

I haven't analyzed why it started to happen

It probably started to occur as a result of #946

@yarikoptic
Copy link
Member

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
Copy link
Member

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
Copy link
Member Author

jwodder commented Jun 15, 2022

This was resolved some time ago.

@jwodder jwodder closed this as completed Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants