Skip to content

⚡♻️ Consolidate and speed up delete method#36

Merged
fiendish merged 2 commits intomasterfrom
fixreadme
Jun 18, 2021
Merged

⚡♻️ Consolidate and speed up delete method#36
fiendish merged 2 commits intomasterfrom
fixreadme

Conversation

@fiendish
Copy link
Contributor

@fiendish fiendish commented Jun 4, 2021

No description provided.

@fiendish
Copy link
Contributor Author

fiendish commented Jun 4, 2021

@chris-s-friedman Can you test this reimplemented delete function to see if it runs better?

Copy link
Contributor

@chris-s-friedman chris-s-friedman left a comment

Choose a reason for hiding this comment

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

i had an issue where it stopped deleting things after 248 sequencing experiments got deleted

delete_kfids(host, STUDIES, [study_id], safety_check=safety_check)
)
kfids = yield_kfids(host, endpoint, {"study_id": study_id})
delete_kfids(host, kfids, safety_check=safety_check)
Copy link
Contributor

Choose a reason for hiding this comment

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

delete_kfids prints how many things are being deleted at the current endpoint. there should be a statement before deleting printing what endpoint is currently being deleted e.g:

Suggested change
delete_kfids(host, kfids, safety_check=safety_check)
print(f"Deleting kfids from {endpoint}.")
delete_kfids(host, kfids, safety_check=safety_check)

Otherwise it's not clear where in the process of deleting the script is.

@fiendish fiendish force-pushed the fixreadme branch 4 times, most recently from 7c15130 to f030fa6 Compare June 16, 2021 15:11
Copy link
Contributor

@chris-s-friedman chris-s-friedman left a comment

Choose a reason for hiding this comment

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

I'm still getting an issue where delete_kfids gets to the 248th item and then stops.

Comment on lines 107 to 110
print(f"Finding all {endpoint}.")
kfids = yield_kfids(host, endpoint, {}, show_progress=True)
print(f"Deleting all {endpoint}.")
delete_kfids(host, kfids, safety_check=safety_check)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm now getting

Deleting all studies from http://localhost:5000
Finding all read-groups.
Deleting all read-groups.
oFinding all read-group-genomic-files.
Deleting all read-group-genomic-files.
oFinding all sequencing-experiments.
Deleting all sequencing-experiments.
oFinding all sequencing-experiment-genomic-files.
Deleting all sequencing-experiment-genomic-files.
oFinding all genomic-files.
Deleting all genomic-files.

i'm thinking that leading o is printed from delete_kfids?
maybe something like below so that we don't try to delete nothing?

Suggested change
print(f"Finding all {endpoint}.")
kfids = yield_kfids(host, endpoint, {}, show_progress=True)
print(f"Deleting all {endpoint}.")
delete_kfids(host, kfids, safety_check=safety_check)
print(f"Finding all {endpoint}.")
kfids = yield_kfids(host, endpoint, {}, show_progress=True)
if len(kfids) > 0:
print(f"Deleting all {endpoint}.")
delete_kfids(host, kfids, safety_check=safety_check)
else:
print(f"Nothing found at {endpoint}.")

Copy link
Contributor Author

@fiendish fiendish Jun 17, 2021

Choose a reason for hiding this comment

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

ok try again. you'll have to install dependencies again because I added a new one for cleaner scraping progress display

@fiendish fiendish force-pushed the fixreadme branch 3 times, most recently from 0d8a35b to 07f25e0 Compare June 17, 2021 02:15
Copy link
Contributor

@chris-s-friedman chris-s-friedman left a comment

Choose a reason for hiding this comment

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

Looks great!

@fiendish fiendish merged commit 089b62f into master Jun 18, 2021
@fiendish fiendish deleted the fixreadme branch June 18, 2021 19:05
fiendish added a commit that referenced this pull request Jun 18, 2021
## Release 0.4.0

### Summary

- Emojis: ♻️ x1, ⚡ x2, ✅ x1, 🧐 x1, ✨ x2
- Categories: Additions x2, Other Changes x5

### New features and changes

- [#36](#36) - ⚡♻️ Consolidate and speed up delete method - [089b62f](089b62f) by [fiendish](https://github.com/fiendish)
- [#35](#35) - ✅ Reduce size of some tests - [b143fd9](b143fd9) by [fiendish](https://github.com/fiendish)
- [#34](#34) - ⚡ Only patch descendant visibility if needed (also adds dry_run) - [196ee2b](196ee2b) by [fiendish](https://github.com/fiendish)
- [#33](#33) - 🧐 Populate starting entities in descendant tree - [a696fc5](a696fc5) by [fiendish](https://github.com/fiendish)
- [#32](#32) - ✨ Add whole entity hiding function to only hide visible entities - [77bc428](77bc428) by [fiendish](https://github.com/fiendish)
- [#31](#31) - ✨ Add yielding entity contents from kfids - [7420fa6](7420fa6) by [fiendish](https://github.com/fiendish)
@fiendish fiendish mentioned this pull request Jun 18, 2021
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

Successfully merging this pull request may close these issues.

2 participants