-
Notifications
You must be signed in to change notification settings - Fork 8
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
Support publish dataset with update current version #166
Conversation
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.
This looks good! The only thing I could think of to add is tests for permissions, but I realize that permissions in general is something that we are going to be addressing seperarately. So I just suggested a mention of the superuser account in the use case.
|
||
is successful, the use case does not return the dataset object, but the HTTP status code `200`. Otherwise, it throws an error.\ | ||
If you want to perform a major version update, you must set the `versionUpdateType` parameter to `VersionUpdateType.MAJOR`.\ | ||
Superusers can pass `VersionUpdateType.UPDATE_CURRENT` to update metadata without changing the version number. This will overwrite the latest published version and therefore will only work for a dataset that has been published at least once. \*Note that this will only work also if there were no file changes in the update.\ | ||
The `datasetId` parameter can be a string, for persistent identifiers, or a number, for numeric identifiers. |
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.
I know we haven't worked on permissions yet generally, but it could be good to add a comment that UPDATE_CURRENT is only available to 'superuser' accounts, since it's more of a special case than other permissions logic. But if we want to wait till later, that's ok.
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.
Hi @ekraffmiller , I think that is exactly what I added in the docs here:
Superusers can pass
VersionUpdateType.UPDATE_CURRENT
to update metadata without changing the version number. This will overwrite the latest published version and therefore will only work for a dataset that has been published at least once. *Note that this will only work also if there were no file changes in the update.
Or were you thinking about something different? I ask about this on the dv-rearchitecture-frontend channel.
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.
Oh, I'm sorry, I misread that comment in the doc! Yes, that looks perfect. :)
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.
Looks good!
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.
LGTM
What this PR does / why we need it:
Adds support to publish dataset use case updating current version for superusers.
Which issue(s) this PR closes:
Special notes for your reviewer:
The
.gitignore
modification is to ignore my vscode folder on my end where I'm using settings to auto format on save with prettier.Suggestions on how to test this:
Visual inspection and run tests.
Is there a release notes update needed for this change?:
N/A
Additional documentation:
N/A