Conversation
9140776 to
7ad36cb
Compare
dbebda5 to
6146861
Compare
|
@mvandenburgh Why do we need the following line? dandi-archive/web/src/utils/schema.ts Line 13 in 2ff1bc7 The JSON schema of |
|
@mvandenburgh I don't think the needs to be replaced because it is a shortened URL for the |
|
@yarikoptic Should be bother to replace the hardcoded It is one of the default supported licenses, and |
a4f4df9 to
a773959
Compare
@yarikoptic Never mind, I replaced the hardcoded license. |
|
following up to the question to @mvandenburgh . So that TODO was added in which added (2nd line unrelated, included for completness) and it didn't reference any particular PR or even dandi-schema version. Looking at that commit state of dandi-schema it was so used 0.6.9 schema. And the diff between the two is so it seems that the 0.6.10 had fixed the pattern and we can just remove that ad-hoc fixup of the schema. @mvandenburgh please confirm but meanwhile we proceed under such an assumption |
In our discussion about this, @yarikoptic concurred, so let's proceed with my initial judgement. |
ff5b077 to
b9b2775
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR updates the DANDI archive codebase to utilize vendor-specific information from schema instance configuration instead of hardcoded values. The changes replace hardcoded DANDI: prefixes, RRID identifiers, and license lists with corresponding attributes from dandischema.conf.
- Replaces hardcoded instance-specific strings with configuration-driven values
- Updates metadata generation to use schema instance configuration
- Adds frontend support for dynamic instance naming
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| web/src/types/index.ts | Adds interface definition for instance configuration |
| web/src/components/DandisetList.vue | Updates to display dynamic archive name instead of hardcoded "DANDI" |
| dev/.env.docker-compose-native | Adds environment variable for instance name |
| dandiapi/settings/base.py | Imports and configures schema instance configuration |
| dandiapi/api/views/dandiset.py | Updates identifier prefix removal to use dynamic instance name |
| dandiapi/api/tests/test_version.py | Updates test assertions to use schema configuration values |
| dandiapi/api/tests/test_tasks.py | Updates test assertions for publish task metadata |
| dandiapi/api/tests/test_asset.py | Updates test assertions for asset publishing |
| dandiapi/api/services/metadata/init.py | Updates metadata ID generation to use dynamic instance name |
| dandiapi/api/models/version.py | Updates version metadata to use configuration-driven identifiers |
| dandiapi/api/models/metadata.py | Updates publish metadata to use dynamic instance configuration |
| dandiapi/api/management/commands/create_dev_dandiset.py | Updates license selection to use configured licenses |
| .github/workflows/frontend-ci.yml | Adds instance name environment variable |
| .github/workflows/cli-integration.yml | Updates to use temporary test branch |
| .github/workflows/backend-ci.yml | Adds instance configuration environment variables |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
To include the output of instance config value
b6104d2 to
689faaa
Compare
9c36d76 to
4ebd692
Compare
35dd1a5 to
ca9a214
Compare
|
@candleindark I believe from our end this is all set. The failing CLI tests will are fixed by dandi/dandi-cli#1739 (confirmed by this test run aimed at the CLI branch). At this point, we're just waiting for the dandi-schema PR to be merged and released. Then, we can proceed with this PR (dropping the temporary commits and merging). |
jjnesbitt
left a comment
There was a problem hiding this comment.
We'll just need to make sure to squash when we merge.
|
FWIW - https://pypi.org/project/dandischema/0.12.0/ is now out. Test against it?! |
|
The CLI integration tests are failing, which is expected and okay, since all of the tests see a schema version of Per this comment, we will merge this PR first, and then the CLI PR. |
|
🚀 PR was released in |
This PR includes all the relevant commits (first 4) in #2430. Checkout #2430 for changes regarding those commits.
Additionally, this PR closes #2382 with the following changes.
Additionally, it replaces the hardcoded
DAND:, the hardcoded RRID, and the supported licenses with the corresponding attributes in schema instance config defined indandischema.conf.Particularly, the hardcoded
DAND:that are handled aregit grep DANDI: -- dandiapi | grep -v -e test_ -e 'subject=' -e 'verbose_name'dandiapi/api/models/version.py: 'identifier': f'DANDI:{self.dandiset.identifier}',dandiapi/api/models/version.py: 'id': f'DANDI:{self.dandiset.identifier}/{self.version}',dandiapi/api/services/metadata/__init__.py: f'DANDI:{publishable_version.dandiset.identifier}/{publishable_version.version}'git grep DANDI: -- web | grep -v -e test_ -e 'subject=' -e 'verbose_name'web/src/components/DandisetList.vue: DANDI:<b>{{ item.dandiset.identifier }}</b>web/src/utils/schema.ts: deepCopySchema['properties']['identifier']['pattern'] = '^DANDI:\\d{6}$'web/src/views/DandisetLandingView/DownloadDialog.vue: // Use the special 'DANDI:' url prefix if appropriate.web/src/views/DandisetLandingView/DownloadDialog.vue: const dandiUrl = `DANDI:${identifier}`;https://www.dandiarchive.org/. The hardcode doesn't need to be replaced. Make sure the containing functiondownloadCommandworks for other instances of DANDI though. (See Add vendorization support #2584 (comment) for details)TODO:
settings.DANDI_SCHEMA_INSTANCE_CONFIG.instance_identifierisNoneinmetadata.pydandischemadependency point to its release version of0.12.0once Vendor-Configurable Metadata Models dandi-schema#294 is merged and released as version of0.12.0Note
api/info/endpoint provide instance config info #2430 and is targeted to be merged to the corresponding branch of Have theapi/info/endpoint provide instance config info #2430.Incidentally, this PR also makes the following minor changes.
PublishActivityby using the version of the dandi-archive instead of a hardcoded version number.Release Notes
This PR makes the instance config as defined in
dandischema.conf, per dandi/dandi-schema#294, available through theapi/info/endpoint and replaces the hardcodedDAND:, the hardcoded RRID, and the supported licenses with the corresponding attributes in schema instance config defined indandischema.conf.