Skip to content

✨ Delete dataservice entities#24

Merged
fiendish merged 5 commits into
masterfrom
delete-study
Mar 29, 2021
Merged

✨ Delete dataservice entities#24
fiendish merged 5 commits into
masterfrom
delete-study

Conversation

@znatty22

@znatty22 znatty22 commented Mar 19, 2021

Copy link
Copy Markdown
Member

Closes #23

Delete entities in Dataservice by study id or blanket delete all entities in Dataservice

@znatty22 znatty22 added the enhancement New feature or request label Mar 19, 2021
@znatty22 znatty22 self-assigned this Mar 19, 2021
@znatty22 znatty22 changed the title ✨ Delete Dataservice entities ✨ Delete dataservice entities Mar 19, 2021
Comment thread tests/conftest.py

DATASERVICE_URL="http://localhost:5000"

@pytest.fixture

@znatty22 znatty22 Mar 19, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this seems silly for testing delete_entities since this fixture uses the thing we're going to test lol, but we need some way to cleanup the dataservice for all of the integration tests that would make use of the live dataservice. That's also why I added unit tests for delete_entities that make use of mocks and don't depend on the live dataservice.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol no I love it

@fiendish

fiendish commented Mar 22, 2021

Copy link
Copy Markdown
Contributor

You don't want to just repeatedly call DELETE on the study endpoint until the data service stops erroring? 🀣

@znatty22

Copy link
Copy Markdown
Member Author

You don't want to just repeatedly call DELETE on the study endpoint until the data service stops erroring? 🀣

Lol nooo, that would crash it for the big studies esp the ones that have the cohort VCFs

@znatty22 znatty22 force-pushed the delete-study branch 5 times, most recently from d1bee79 to a78a59c Compare March 25, 2021 19:28
@znatty22 znatty22 marked this pull request as ready for review March 25, 2021 19:36
Comment thread kf_utils/dataservice/delete.py Outdated
"""
session = Session()
errors = {}
for i, kf_id in enumerate(kfids):

@fiendish fiendish Mar 29, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like the i from enumerate is being used. Is something missing here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nothing changed lol

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh i see, oops. One sec

Comment thread kf_utils/dataservice/delete.py Outdated
"participants",
"family-relationships",
"families",
"studies",

@fiendish fiendish Mar 29, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a comment, not part of the change request...

It seems like the 'studies' entry in ENDPOINTS is only used by one test. It might make more sense to put studies into its own constant and not include it here and then add a separate studies endpoint check in that one test so that you don't have to have [:-1] everywhere.

@znatty22 znatty22 requested a review from fiendish March 29, 2021 18:55
@znatty22 znatty22 force-pushed the delete-study branch 2 times, most recently from 12b7220 to 599327b Compare March 29, 2021 19:18
@fiendish fiendish merged commit e48dbbd into master Mar 29, 2021
@fiendish fiendish deleted the delete-study branch March 29, 2021 20:02
fiendish added a commit that referenced this pull request Mar 30, 2021
## Release 0.3.0

### Summary

- Emojis: πŸ‘· x2, ✨ x2
- Categories: Additions x2, Ops x2

### New features and changes

- [#25](#25) - πŸ‘· Add release workflow  6a4fe6e - [96e5f65](96e5f65) by [fiendish](https://github.com/fiendish)
- [#24](#24) - ✨ Delete dataservice entities - [e48dbbd](e48dbbd) by [znatty22](https://github.com/znatty22)
- [#22](#22) - πŸ‘· Add CI config - [82864c0](82864c0) by [fiendish](https://github.com/fiendish)
- [#20](#20) - ✨ Add function for reporting S3 and DS GF entries - [f99f2cd](f99f2cd) by [fiendish](https://github.com/fiendish)
@fiendish fiendish mentioned this pull request Mar 30, 2021
fiendish added a commit that referenced this pull request Mar 30, 2021
## Release 0.3.0

### Summary

- Emojis: πŸ‘· x2, ✨ x2
- Categories: Additions x2, Ops x2

### New features and changes

- [#25](#25) - πŸ‘· Add release workflow - [96e5f65](96e5f65) by [fiendish](https://github.com/fiendish)
- [#24](#24) - ✨ Delete dataservice entities - [e48dbbd](e48dbbd) by [znatty22](https://github.com/znatty22)
- [#22](#22) - πŸ‘· Add CI config - [82864c0](82864c0) by [fiendish](https://github.com/fiendish)
- [#20](#20) - ✨ Add function for reporting S3 and DS GF entries - [f99f2cd](f99f2cd) by [fiendish](https://github.com/fiendish)
@fiendish fiendish mentioned this pull request Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Delete dataservice entities

2 participants