-
Notifications
You must be signed in to change notification settings - Fork 11
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
Disallow extra fields other than "@context"
#266
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #266 +/- ##
==========================================
- Coverage 97.71% 92.21% -5.51%
==========================================
Files 16 16
Lines 1753 1772 +19
==========================================
- Hits 1713 1634 -79
- Misses 40 138 +98
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
05e6307
to
241bc08
Compare
…t` and `Dandiset` This requires `"@context"` field at the JSON level
This also sets the `"additionalProperties"` of each corresponding model in JSON schema to `false`
@satra @mvandenburgh This solution will generated JSON schemas that have |
are failing tests expected? @mvandenburgh please point @candleindark to where to change URL to the json schema so some alternative one could be tested. |
@candleindark the json schema used by the frontend is set here - https://github.com/dandi/dandi-archive/blob/master/dandiapi/api/views/info.py#L11-L14 |
Yes. Because I made |
These are not made available in `__all__`. Use of them triggers complains in the IDE.
@@ -1815,6 +1867,12 @@ class Asset(BareAsset): | |||
json_schema_extra={"readOnly": True, "nskey": "schema"} | |||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
look here for an example on how we add extra to json schema.
May be you could add extra for your @context
which would state something like "includeInUI": False
This is needed to handle the case in which the keys are newly created
f9bba62
to
99a66ed
Compare
@mvandenburgh I completed the initial setup, started the API server using On a related note, I was wondering if the web app/UI depends on the |
I just tried the native development setup with |
The web UI only depends on
Does the web UI always display that "Connection to server failed" message, or just when you modify |
It is the latter. I got the attached landing page if nothing no code is modified. In fact, there is no problem even if I hardcode the schema_url = (
'https://raw.githubusercontent.com/dandi/schema/master/'
f'releases/0.6.8/dandiset.json'
) or even schema_url = (
'https://raw.githubusercontent.com/dandi/schema/master/'
f'releases/0.6.7/dandiset.json'
) The problem occurred when |
This PR closes #75. It addresses #75 in the follow manner.
"@context"
field in both Pydantic and JSON level."@context"
field can be validated against a Pydantic model but ignored.The solution implemented in this PR is based on the following script.
TODOs
@context
optional and then discuss*/*/dandiset.jsonld
files under/mnt/backup/dandi/dandiset-manifests-s3cmd
.