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

Dataset locks API #4926

Closed
kcondon opened this issue Aug 7, 2018 · 9 comments
Closed

Dataset locks API #4926

kcondon opened this issue Aug 7, 2018 · 9 comments
Assignees

Comments

@kcondon
Copy link
Contributor

kcondon commented Aug 7, 2018

There may be times when a dataset lock becomes disassociated/orphaned/stuck and needs to be cleared. Creating an endpoint would make it easier than going to the db and doing it. Latest use case is file doi registration is interrupted, leaving lock in place.

@djbrooke djbrooke self-assigned this Aug 7, 2018
@djbrooke djbrooke changed the title Unlock Dataset: Create API endpoint to allow superusers to unlock a dataset Unlock Dataset: Create API admin endpoint to allow administrators to unlock a dataset Aug 8, 2018
@djbrooke djbrooke changed the title Unlock Dataset: Create API admin endpoint to allow administrators to unlock a dataset Unlock Dataset: Create API admin endpoint to allow administrators to unlock a specific dataset lock Aug 8, 2018
@djbrooke djbrooke removed their assignment Aug 8, 2018
@landreev landreev self-assigned this Aug 16, 2018
@landreev
Copy link
Contributor

Note, as discussed last week, in addition to the API call that deletes a lock, also implemented API calls to look up/check locks, and to lock datasets (as DELETE, GET and POST end points).
(plus the IT tests for these APIs)

landreev added a commit that referenced this issue Aug 20, 2018
Added superuser-only requirement for the POST and DELETE methods. (#4926)
@landreev landreev mentioned this issue Aug 20, 2018
5 tasks
@landreev landreev changed the title Unlock Dataset: Create API admin endpoint to allow administrators to unlock a specific dataset lock Dataset locks API Aug 20, 2018
@landreev landreev removed their assignment Aug 20, 2018
@pdurbin
Copy link
Member

pdurbin commented Aug 20, 2018

Pull request #4986 looks good to me. Good docs. Happy to see tests. I moved it over to QA but I did leave this comment in my review: "The only thing that gave me pause is that no authentication seems to be necessary to check for locks but I don't think this would cause much harm. Approved."

@scolapasta
Copy link
Contributor

Agree with @pdurbin about not much harm, but technically it could allow a user to learn info about a draft dataset which they shouldn't know exists.

@pameyer
Copy link
Contributor

pameyer commented Aug 20, 2018

@scolapasta From the docs, this info would appear to be retrieving the lock type of an unpublished dataset for which the user already knows a DOI - authentication should still be necessary to retrieve metadata or files; so I'd agree that this isn't likely to be a problem.

@oscardssmith
Copy link
Contributor

why not just tie this to view permissions?

@pdurbin
Copy link
Member

pdurbin commented Aug 24, 2018

@kcondon and I noticed that the PDF is failing to build for the guides using the "guides.dataverse.org-rfi" job in Jenkins. The branch was 31 commits behind the "develop" branch...

screen shot 2018-08-24 at 12 32 57 pm

... so in 3e611e0 I just merged the latest from "develop" into the "4926-unlock-dataset-api" branch so we can try again. This failure to build the PDF was first noticed for #4973 which was also a bit behind "develop". "develop" is building the PDF fine with the same job so hopefully this fixes it.

@kcondon
Copy link
Contributor Author

kcondon commented Aug 27, 2018

  • @landreev One quick issue, when deleting the InReview lock, the dataset card needs to be reindexed, otherwise it shows InReview when it is not.

Another issue: when deleting all locks on a dataset that has multiple locks, it deletes one, then throws a 500 error.

  • Another minor issue, a typo in the manual lists lock type pidRegistration in the example when it should be pidRegister

@landreev
Copy link
Contributor

Will check in fix for the indexing part (and the typo fix in the guide) momentarily.

@landreev landreev removed their assignment Aug 30, 2018
landreev added a commit that referenced this issue Aug 30, 2018
…ing multiple locks, to avoid concurrent exceptions. (#4926)
@landreev
Copy link
Contributor

Fixed: reindexing; concurrent modification exception w/ multiple locks; typo in the guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants