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

Set the $schema key with the schema dialect #236

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

candleindark
Copy link
Member

This PR closes #216.

Copy link

codecov bot commented Apr 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.83%. Comparing base (57b503a) to head (5841e4e).
Report is 12 commits behind head on master.

❗ Current head 5841e4e differs from pull request most recent head 862f25f. Consider uploading reports for the commit 862f25f to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #236      +/-   ##
==========================================
- Coverage   97.58%   91.83%   -5.76%     
==========================================
  Files          16       16              
  Lines        1701     1727      +26     
==========================================
- Hits         1660     1586      -74     
- Misses         41      141     +100     
Flag Coverage Δ
unittests 91.83% <100.00%> (-5.76%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@candleindark candleindark marked this pull request as ready for review April 15, 2024 14:26
@candleindark
Copy link
Member Author

candleindark commented Apr 15, 2024

@yarikoptic @satra Adding the $schema key will change the resulting schemas. Should we up the version to 0.6.8?

@yarikoptic
Copy link
Member

I think it is indeed would be desired. Please add an explanation as well as you did in 09e50e1

@yarikoptic yarikoptic added the patch Increment the patch version when merged label Apr 15, 2024
@yarikoptic
Copy link
Member

although may be we should just set it upon adding a PR which has release label -- that is when our "Test that old schemata are not modified" kicks in as well?

@candleindark candleindark added release Create a release when this pr is merged and removed release Create a release when this pr is merged labels Apr 15, 2024
@candleindark
Copy link
Member Author

@yarikoptic I updated the DANDI_SCHEMA_VERSION to "0.6.8". I am not sure if this PR should have the release label. I don't see it as a major change though it alters the generated schemas.

@yarikoptic
Copy link
Member

let's drop last commit and proceed without releasing for now

git reset --hard HEAD^
git push -f

@candleindark candleindark force-pushed the with-schema-dialect branch 3 times, most recently from a783271 to 862f25f Compare April 23, 2024 18:51
@candleindark candleindark removed the patch Increment the patch version when merged label Apr 23, 2024
@candleindark
Copy link
Member Author

candleindark commented Apr 23, 2024

@yarikoptic I trimmed this PR down to the only essential commit. However, for some reason, some of the cli tests failed at the following.

FAILED tests/test_download.py::test_download_newest_version - ValueError: Dandiset 000077 is Invalid: {
    "asset_validation_errors": [],
    "version_validation_errors": [
        {
            "field": "contributor",
            "message": "Value error, Contact person must have an email address."
        },
        {
            "field": "contributor",
            "message": "Input should be 'Organization'"
        }
    ]
}

That error is only included in dc15401 which is not yet merged with this PR. How does this come about?

The other error is

  Version 3.8 was not found in the local cache
  Error: The version '3.8' with architecture 'arm64' was not found for macOS 14.4.1.
  The list of all available versions can be found here: https://raw.githubusercontent.com/actions/python-versions/main/versions-manifest.json

It seems that Python 3.8 is no longer available for the latests macOS version.

@yarikoptic
Copy link
Member

pull_request is tested after merging master which has #235 merged now, and apparently I didn't mention that dandi-cli tests were failing there whenever I clicked "Merge" . See e.g. https://github.com/dandi/dandi-schema/actions/runs/8696464081/job/23849776507 so we need to fix them in dandi-cli now. Filed

@yarikoptic
Copy link
Member

The other error is

  Version 3.8 was not found in the local cache
  Error: The version '3.8' with architecture 'arm64' was not found for macOS 14.4.1.
  The list of all available versions can be found here: https://raw.githubusercontent.com/actions/python-versions/main/versions-manifest.json

it seems also for 3.9 . But note that it is arm64 -- I wonder if they just switched...

looking at our dump of logs we keep on drogon:

dandi@drogon:/mnt/backup/dandi/tinuous-logs/dandischema/2024/04$ ls -ld */github/cron/*/*/test.yml/*/*macos*3.8*
...
drwxr-sr-x 1 dandi dandi   436 Apr 16 02:20 '16/github/cron/20240416T060243/d46af61/test.yml/2035-success/test (macos-latest, 3.8)'
-rw-r--r-- 1 dandi dandi 49462 Apr 17 02:20 '17/github/cron/20240417T060248/d46af61/test.yml/2036-success/10_test (macos-latest, 3.8).txt'
drwxr-sr-x 1 dandi dandi   436 Apr 17 02:20 '17/github/cron/20240417T060248/d46af61/test.yml/2036-success/test (macos-latest, 3.8)'
-rw-r--r-- 1 dandi dandi 55340 Apr 18 02:20 '18/github/cron/20240418T060235/dc15401/test.yml/2039-failed/10_test (macos-latest, 3.8).txt'
drwxr-sr-x 1 dandi dandi   314 Apr 18 02:20 '18/github/cron/20240418T060235/dc15401/test.yml/2039-failed/test (macos-latest, 3.8)'
...

so it flipped around 18th... but the fail there is not what you quoted, it is again about this new aspect of requiring email for contact person including the last one from today:

2024-04-23T06:04:51.7209590Z ____________________________ test_datacite[000008] _____________________________
2024-04-23T06:04:51.7218140Z dandischema/tests/test_datacite.py:157: in test_datacite
2024-04-23T06:04:51.7219320Z     meta = PublishedDandiset(**meta_js)
2024-04-23T06:04:51.7221660Z E   pydantic_core._pydantic_core.ValidationError: 5 validation errors for PublishedDandiset
2024-04-23T06:04:51.7224590Z E   contributor.0.function-after[ensure_contact_person_has_email(), Person]
2024-04-23T06:04:51.7226380Z E     Value error, Contact person must have an email address. [type=value_error, input_value={'schemaKey': 'Person', '...ncludeInCitation': True}, input_type=dict]
2024-04-23T06:04:51.7227760Z E       For further information visit https://errors.pydantic.dev/2.7/v/value_error
2024-04-23T06:04:51.7229220Z E   contributor.0.function-after[ensure_contact_person_has_email(), Organization].schemaKey
2024-04-23T06:04:51.7230460Z E     Input should be 'Organization' [type=literal_error, input_value='Person', input_type=str]
2024-04-23T06:04:51.7231510Z E       For further information visit https://errors.pydantic.dev/2.7/v/literal_error
2024-04-23T06:04:51.7232650Z E   contributor.16.function-after[ensure_contact_person_has_email(), Person]
2024-04-23T06:04:51.7234200Z E     Value error, Contact person must have an email address. [type=value_error, input_value={'schemaKey': 'Person', '...ncludeInCitation': True}, input_type=dict]
2024-04-23T06:04:51.7235530Z E       For further information visit https://errors.pydantic.dev/2.7/v/value_error
2024-04-23T06:04:51.7236690Z E   contributor.16.function-after[ensure_contact_person_has_email(), Organization].schemaKey
2024-04-23T06:04:51.7237910Z E     Input should be 'Organization' [type=literal_error, input_value='Person', input_type=str]
2024-04-23T06:04:51.7239290Z E       For further information visit https://errors.pydantic.dev/2.7/v/literal_error
2024-04-23T06:04:51.7240640Z E   contributor.16.function-after[ensure_contact_person_has_email(), Organization].identifier
2024-04-23T06:04:51.7242170Z E     String should match pattern '^https://ror.org/[a-z0-9]+$' [type=string_pattern_mismatch, input_value='0000-0002-4305-6376', input_type=str]
2024-04-23T06:04:51.7243730Z E       For further information visit https://errors.pydantic.dev/2.7/v/string_pattern_mismatch
2024-04-23T06:04:51.7244900Z __________ test_dandimeta_datacite[additional_meta1-datacite_checks1] __________
2024-04-23T06:04:51.7245780Z dandischema/tests/test_datacite.py:385: in test_dandimeta_datacite
2024-04-23T06:04:51.7246490Z     datacite = to_datacite(metadata_basic)
2024-04-23T06:04:51.7247030Z dandischema/datacite.py:66: in to_datacite
2024-04-23T06:04:51.7247520Z     meta = PublishedDandiset(**meta)
2024-04-23T06:04:51.7248300Z E   pydantic_core._pydantic_core.ValidationError: 2 validation errors for PublishedDandiset
2024-04-23T06:04:51.7249380Z E   contributor.0.function-after[ensure_contact_person_has_email(), Person]
2024-04-23T06:04:51.7250830Z E     Value error, Contact person must have an email address. [type=value_error, input_value={'name': 'A_last, A_first...'dcite:ContactPerson'>]}, input_type=dict]
2024-04-23T06:04:51.7252110Z E       For further information visit https://errors.pydantic.dev/2.7/v/value_error
2024-04-23T06:04:51.7253120Z E   contributor.0.function-after[ensure_contact_person_has_email(), Organization]
2024-04-23T06:04:51.7254590Z E     Value error, Contact person must have an email address. [type=value_error, input_value={'name': 'A_last, A_first...'dcite:ContactPerson'>]}, input_type=dict]
2024-04-23T06:04:51.7255860Z E       For further information visit https://errors.pydantic.dev/2.7/v/value_error
2024-04-23T06:04:51.7256840Z __________ test_dandimeta_datacite[additional_meta2-datacite_checks2] __________
...

which adds to the insult of me missing all those fails while merging... Do you think you could look into them asap or should I?

re arm64, I see it emerged only today in this PR:

23/github/pr/236/862f25f/test-dandi-cli.yml/1447-failed/4_test-dandi-cli (macos-latest, 3.8, master, normal).txt:2024-04-23T18:53:10.1224310Z Image: macos-14-arm64

and that flipped today -- before that it was macos-12:

dandi@drogon:/mnt/backup/dandi/tinuous-logs/dandischema/2024/04$ git grep 'Image:'  -- '*/test.yml/*/10_test (macos-latest, 3.8).txt'
01/github/cron/20240401T060245/57b503a/test.yml/2006-success/10_test (macos-latest, 3.8).txt:2024-04-01T06:02:53.7957430Z Image: macos-12
...
22/github/cron/20240422T060246/dc15401/test.yml/2043-failed/10_test (macos-latest, 3.8).txt:2024-04-22T06:02:54.5086340Z Image: macos-12
23/github/cron/20240423T060233/dc15401/test.yml/2044-failed/10_test (macos-latest, 3.8).txt:2024-04-23T06:03:55.7784620Z Image: macos-12
23/github/pr/236/862f25f/test.yml/2048-failed/10_test (macos-latest, 3.8).txt:2024-04-23T18:52:54.4118480Z Image: macos-14-arm64
23/github/pr/236/862f25f/test.yml/2050-failed/10_test (macos-latest, 3.8).txt:2024-04-23T18:59:26.5157860Z Image: macos-14-arm64

let's see if

@candleindark
Copy link
Member Author

Those errors are very strange, but I think will try to resolve dandi/dandi-cli#1430 first.

@yarikoptic yarikoptic added the patch Increment the patch version when merged label Apr 30, 2024
@yarikoptic yarikoptic merged commit 2b4eacd into dandi:master Apr 30, 2024
107 of 128 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a top-level $schema field to the generated JSON Schemata
2 participants