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.

Sign up for GitHub

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

Jump to bottom

Upgrade dandischema to 0.7.x series #987

Merged
merged 2 commits into from
May 5, 2022
Merged

Upgrade dandischema to 0.7.x series #987

merged 2 commits into from
May 5, 2022

Conversation

yarikoptic
Copy link
Member

per @satra request

@yarikoptic yarikoptic added release Create a release when this pr is merged internal Changes only affect the internal API labels Apr 26, 2022
@satra
Copy link
Member

satra commented Apr 26, 2022

the errors seem to be related to the url field. since the schema was changed to support a couple of external environment variables, perhaps some additional change is needed in the CLI tests.

@jwodder
Copy link
Member

jwodder commented Apr 26, 2022

cf. dandi/dandi-schema#132

@yarikoptic
Copy link
Member Author

@jwodder if you see what to do please proceed

@jwodder
Copy link
Member

jwodder commented Apr 27, 2022

Even after updating the metadata test cases, the publication tests are still failing with "Metadata version 0.6.3 is not allowed. Allowed are: 0.6.2, 0.4.4, 0.5.1, 0.5.2, 0.6.0, 0.6.1.", likely due to the fact that dandi-archive is still using dandischema 0.6.0.

@codecov
Copy link

codecov bot commented Apr 27, 2022 •
edited
Loading

Codecov Report

Merging #987 (9a74940) into master (5154ca3) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #987   +/-   ##
=======================================
  Coverage   87.50%   87.50%           
=======================================
  Files          64       64           
  Lines        8106     8122   +16     
=======================================
+ Hits         7093     7107   +14     
- Misses       1013     1015    +2
Flag Coverage Δ
unittests 87.50% <ø> (ø)

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

Impacted Files Coverage Δ
dandi/bids_validator_xs.py 92.61% <0.00%> (-0.30%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d54491...9a74940. Read the comment docs.

@yarikoptic yarikoptic mentioned this pull request Apr 27, 2022
Merged
@yarikoptic
Copy link
Member Author

Even after updating the metadata test cases, the publication tests are still failing with "Metadata version 0.6.3 is not allowed. Allowed are: 0.6.2, 0.4.4, 0.5.1, 0.5.2, 0.6.0, 0.6.1.", likely due to the fact that dandi-archive is still using dandischema 0.6.0.

indeed it does:

(git)lena:~/proj/dandi/dandi-archive[master]git
$> git describe
v0.2.17-4-gfe7b79cb

$> git grep dandischema -- setup*
setup.py:        'dandischema==0.6.0',

since there is metadata auto-upgrading on server side, shouldn't first dandi-archive to gain upgrade to 0.7.0 series and then we proceed with this PR here @satra, attn @waxlamp @dchiquito

@yarikoptic yarikoptic mentioned this pull request Apr 28, 2022
Merged
@jwodder
Copy link
Member

jwodder commented May 3, 2022

@yarikoptic The tests are passing now.

@yarikoptic
Copy link
Member Author

Cool, but I guess we must not merge/release until we release dandi archive first

This was referenced May 3, 2022
Merged
Closed
@yarikoptic
Copy link
Member Author

ok, archive is deployed. This upgrade is not strictly needed AFAIK since dandischema would just upgrade from 0.6.2 to 0.6.3 on server. But should not hurt. so let's proceed

@yarikoptic yarikoptic merged commit b31d990 into master May 5, 2022
@yarikoptic yarikoptic deleted the rf-upgrade-schema branch May 5, 2022 14:26
@github-actions
Copy link

github-actions bot commented May 5, 2022

🚀 PR was released in 0.39.5 🚀

yarikoptic added a commit to dandi/dandi-schema that referenced this pull request Jun 3, 2022
@yarikoptic
Revert "Set DJANGO_DANDI_WEB_APP_URL for dandi-cli integration tests"
fcda7da 
This reverts commit 32fc8f6.

Long analysis:

test_metadata2asset of dandi-cli fails integration testing started to fail regularly.

First I thought that it started to happen since #126 according to

```
(base) dandi@drogon:/mnt/backup/dandi/tinuous-logs/dandischema/2022/03$ git grep 'FAILED.*test_metadata2asset.*metadata2asset.json-metadata0' | head
24/github/pr/126/e1853d9/test-dandi-cli.yml/471-failed/3_test-dandi-cli (ubuntu-18.04, 3.8, master, normal).txt:2022-03-24T23:23:13.4506531Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
24/github/pr/126/e1853d9/test-dandi-cli.yml/471-failed/5_test-dandi-cli (macos-latest, 3.8, master, normal).txt:2022-03-24T23:24:11.6748940Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
24/github/pr/126/e1853d9/test-dandi-cli.yml/471-failed/7_test-dandi-cli (ubuntu-18.04, 3.8, dandi-devel, master).txt:2022-03-24T23:24:07.6147814Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
24/github/pr/126/e1853d9/test-dandi-cli.yml/471-failed/test-dandi-cli (macos-latest, 3.8, master, normal)/12_Run dandi-cli tests.txt:2022-03-24T23:24:11.6748630Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
24/github/pr/126/e1853d9/test-dandi-cli.yml/471-failed/test-dandi-cli (ubuntu-18.04, 3.8, dandi-devel, master)/12_Run dandi-cli tests.txt:2022-03-24T23:24:07.6147810Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
24/github/pr/126/e1853d9/test-dandi-cli.yml/471-failed/test-dandi-cli (ubuntu-18.04, 3.8, master, normal)/12_Run dandi-cli tests.txt:2022-03-24T23:23:13.4506528Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
25/github/cron/20220325T060244/3e0edbf/test-dandi-cli.yml/472-failed/3_test-dandi-cli (ubuntu-18.04, 3.8, master, normal).txt:2022-03-25T06:07:01.8173971Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
25/github/cron/20220325T060244/3e0edbf/test-dandi-cli.yml/472-failed/5_test-dandi-cli (macos-latest, 3.8, master, normal).txt:2022-03-25T06:06:32.5237510Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
25/github/cron/20220325T060244/3e0edbf/test-dandi-cli.yml/472-failed/7_test-dandi-cli (ubuntu-18.04, 3.8, dandi-devel, master).txt:2022-03-25T06:07:46.3313297Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
25/github/cron/20220325T060244/3e0edbf/test-dandi-cli.yml/472-failed/test-dandi-cli (macos-latest, 3.8, master, normal)/12_Run dandi-cli tests.txt:2022-03-25T06:06:32.5237500Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
```

where it did fail and PR provides little background besides "As discussed in Slack.", then it started to fail until #132 fix where we did set `DJANGO_DANDI_WEB_APP_URL` but it was still an older dandi-cli, then we boosted dandischema in dandi-cli to 0.7.0 in dandi/dandi-cli#987 , released in [0.39.5](https://github.com/dandi/dandi-cli/releases/tag/0.39.5) on May 5th, and we immediately started to fail in cron jobs:

```
(base) dandi@drogon:/mnt/backup/dandi/tinuous-logs/dandischema/2022/05$ git grep 'FAILED.*test_metadata2asset.*metadata2asset.json-metadata0' | grep cron | head
06/github/cron/20220506T060309/34482a2/test-dandi-cli.yml/527-failed/1_test-dandi-cli (windows-2019, 3.8, master, normal).txt:2022-05-06T06:09:25.9624055Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
06/github/cron/20220506T060309/34482a2/test-dandi-cli.yml/527-failed/2_test-dandi-cli (windows-2019, 3.8, release, normal).txt:2022-05-06T06:07:13.7676374Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
06/github/cron/20220506T060309/34482a2/test-dandi-cli.yml/527-failed/3_test-dandi-cli (ubuntu-18.04, 3.8, master, normal).txt:2022-05-06T06:13:43.1372133Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
06/github/cron/20220506T060309/34482a2/test-dandi-cli.yml/527-failed/4_test-dandi-cli (ubuntu-18.04, 3.8, release, normal).txt:2022-05-06T06:11:30.5720453Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
06/github/cron/20220506T060309/34482a2/test-dandi-cli.yml/527-failed/5_test-dandi-cli (macos-latest, 3.8, master, normal).txt:2022-05-06T06:06:44.9864360Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
06/github/cron/20220506T060309/34482a2/test-dandi-cli.yml/527-failed/6_test-dandi-cli (macos-latest, 3.8, release, normal).txt:2022-05-06T06:06:39.2250540Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
06/github/cron/20220506T060309/34482a2/test-dandi-cli.yml/527-failed/7_test-dandi-cli (ubuntu-18.04, 3.8, dandi-devel, master).txt:2022-05-06T06:11:31.5085106Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
06/github/cron/20220506T060309/34482a2/test-dandi-cli.yml/527-failed/8_test-dandi-cli (ubuntu-18.04, 3.8, dandi-devel, release).txt:2022-05-06T06:12:21.3105978Z FAILED tests/test_metadata.py::test_metadata2asset[metadata2asset.json-metadata0]
...
```

the fail report

```
>       assert data.json_dict() == bare_dict
E       AssertionError: assert {'access': [{...n/x-nwb', ...} == {'access': [{...n/x-nwb', ...}
E         Omitting 12 identical items, use -vv to show
E         Left contains 1 more item:
E         {'repository': 'https://dandiarchive.org/'}
E         Full diff:
E           {
E            'access': [{'schemaKey': 'AccessRequirements',
E                        'status': 'dandi:OpenAccess'}],
E            'contentSize': 69105,
E            'digest': {'dandi:dandi-etag': 'e455839e5ab2fa659861f58a423fd17f-1'},
E            'encodingFormat': 'application/x-nwb',
E            'id': 'dandiasset:bfc23fb6192b41c083a7257e09a3702b',
E            'keywords': ['keyword1',
E                         'keyword 2'],
E            'path': '/test/path',
E         +  'repository': 'https://dandiarchive.org/',
E            'schemaKey': 'Asset',
E            'schemaVersion': '0.6.3',
E            'wasAttributedTo': [{'identifier': 'sub-01',
E                                 'schemaKey': 'Participant'}],
E            'wasDerivedFrom': [{'identifier': 'tissue42',
E                                'sampleType': {'name': 'tissuesample',
E                                               'schemaKey': 'SampleType'},
E                                'schemaKey': 'BioSample'}],
E            'wasGeneratedBy': [{'description': 'session_description1',
E                                'identifier': 'session_id1',
E                                'name': 'session_id1',
E                                'schemaKey': 'Session',
E                                'startDate': '2017-04-15T12:00:00+00:00'}],
E           }
```

suggests that it is about the same `repository` not being set  , and that is ok since dandischema no longer sets it unless DJANGO_DANDI_WEB_APP_URL ... dandi-cli relies on the fact that it is not set, and here we set it via env var, so we should stop doing that.
@yarikoptic yarikoptic mentioned this pull request Jun 3, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jwodder jwodder

Labels
internal Changes only affect the internal API release Create a release when this pr is merged released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yarikoptic @satra @jwodder