-
-
Notifications
You must be signed in to change notification settings - Fork 60
Track bytes stored per file type and include in org metrics #1207
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
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
cb2e65f
Add bytes stored per type to org and metrics
tw4l b51af26
Ignore duplicate code in migrations
tw4l 934d7e7
Update tests and drop Bytes from type values in metrics
tw4l 5783d26
Inc bytesStoredCrawls after crawl files stored
tw4l f30b4e2
Update OrgMetrics model for new field names
tw4l 3c62da3
Fix test
tw4l 9a507bd
Make code review revisions
tw4l fa4d78a
Fixup
tw4l 121a312
Make fixes to ensure workflow size is correct after deletion
tw4l b5f073f
Extend tests to test mixed type delete with stats
tw4l b305b72
Add pylint disable comment
tw4l fa55ebd
Fix test fixture
tw4l 08fdbda
Fix migration number in docstring
tw4l fb0616d
Add type check within delete_crawls
tw4l c40ed02
Split crawls and uploads in one filter pass
tw4l e006697
Linting
tw4l 2932284
Add missing await and type hint
tw4l 210843f
Fix getter for crawl type in type check
tw4l File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
73 changes: 73 additions & 0 deletions
73
backend/btrixcloud/migrations/migration_0017_storage_by_type.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| """ | ||
| Migration 0017 - Calculate and store org storage usage by type | ||
| """ | ||
| from btrixcloud.migrations import BaseMigration | ||
|
|
||
|
|
||
| MIGRATION_VERSION = "0017" | ||
|
|
||
|
|
||
| # pylint: disable=too-many-locals, duplicate-code | ||
| class Migration(BaseMigration): | ||
| """Migration class.""" | ||
|
|
||
| def __init__(self, mdb, migration_version=MIGRATION_VERSION): | ||
| super().__init__(mdb, migration_version) | ||
|
|
||
| async def migrate_up(self): | ||
| """Perform migration up. | ||
|
|
||
| Calculate and store org storage usage | ||
| """ | ||
| mdb_orgs = self.mdb["organizations"] | ||
| mdb_crawls = self.mdb["crawls"] | ||
| mdb_profiles = self.mdb["profiles"] | ||
|
|
||
| orgs = [res async for res in mdb_orgs.find({})] | ||
| for org in orgs: | ||
| oid = org.get("_id") | ||
|
|
||
| bytes_stored_crawls = 0 | ||
| bytes_stored_uploads = 0 | ||
| bytes_stored_profiles = 0 | ||
|
|
||
| crawls = [ | ||
| res | ||
| async for res in mdb_crawls.find( | ||
| {"oid": oid, "type": {"$in": [None, "crawl"]}} | ||
| ) | ||
| ] | ||
| for crawl in crawls: | ||
| for crawl_file in crawl.get("files", []): | ||
| bytes_stored_crawls += crawl_file.get("size", 0) | ||
|
|
||
| uploads = [ | ||
| res async for res in mdb_crawls.find({"oid": oid, "type": "upload"}) | ||
| ] | ||
| for upload in uploads: | ||
| for upload_file in upload.get("files", []): | ||
| bytes_stored_uploads += upload_file.get("size", 0) | ||
|
|
||
| profiles = [res async for res in mdb_profiles.find({"oid": oid})] | ||
| for profile in profiles: | ||
| profile_file = profile.get("resource") | ||
| if profile_file: | ||
| bytes_stored_profiles += profile_file.get("size", 0) | ||
|
|
||
| try: | ||
| res = await mdb_orgs.find_one_and_update( | ||
| {"_id": oid}, | ||
| { | ||
| "$set": { | ||
| "bytesStoredCrawls": bytes_stored_crawls, | ||
| "bytesStoredUploads": bytes_stored_uploads, | ||
| "bytesStoredProfiles": bytes_stored_profiles, | ||
| } | ||
| }, | ||
| ) | ||
| # pylint: disable=broad-exception-caught | ||
| except Exception as err: | ||
| print( | ||
| f"Unable to set bytes stored by type for org {oid}: {err}", | ||
| flush=True, | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Can filter by crawl type here, since we have the type available. Files are deleted for all crawls, but then can do:
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.
Actually, it looks like we're not even checking the type before deleting crawl files here!
Should probably do:
This is actually a bug in the current version as well, where it'll delete the files regardless of type, but not actual archived item object!
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.
Good catch on the missing type check! That's now been added.
I think there are advantages to handling the complexity of deleting multiple types in
delete_crawls_all_typesrather thandelete_crawls. Everywhere else in our app that we are deleting content other than through the/all-crawlsdelete endpoint, we're handling one type at a time. And havingdelete_crawlshandle only one type at a time keeps things a bit simpler, e.g. lettinginc_org_bytes_storedonly need to worry about one archived item type at a time.I pushed a change to
delete_crawls_all_typesto split the delete list into crawls and uploads arrays in one pass rather than two, which should help.There are other improvements to be made to the delete endpoints that I'm planning on doing in a separate pass in this sprint (see #1208), so I'd expect some of this to change and get optimized there, but I don't want to hold up this PR for something that's creeping a bit out of scope just because I discovered some bugs 😅 If that sounds reasonable to you!