-
Notifications
You must be signed in to change notification settings - Fork 269
Remove perseus files from storage calculations #5601
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
base: unstable
Are you sure you want to change the base?
Changes from 1 commit
e19fedb
eaba504
d509c37
218f8d7
4e8b3a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -349,20 +349,20 @@ def check_feature_flag(self, flag_name): | |
|
|
||
| def check_channel_space(self, channel): | ||
| tree_cte = With(self.get_user_active_trees().distinct(), name="trees") | ||
| files_cte = With( | ||
| tree_cte.join( | ||
| self.files.get_queryset(), contentnode__tree_id=tree_cte.col.tree_id | ||
| ) | ||
| .values("checksum") | ||
| .distinct(), | ||
| name="files", | ||
| channel_files_qs = tree_cte.join( | ||
| self.files.get_queryset(), contentnode__tree_id=tree_cte.col.tree_id | ||
| ) | ||
| channel_files_qs = self._filter_storage_billable_files(channel_files_qs) | ||
| files_cte = With(channel_files_qs.values("checksum").distinct(), name="files") | ||
|
|
||
| staging_tree_files = ( | ||
| staging_tree_files = self._filter_storage_billable_files( | ||
| self.files.filter(contentnode__tree_id=channel.staging_tree.tree_id) | ||
| .with_cte(tree_cte) | ||
| .with_cte(files_cte) | ||
| .exclude(Exists(files_cte.queryset().filter(checksum=OuterRef("checksum")))) | ||
| ) | ||
| staging_tree_files = staging_tree_files.with_cte(tree_cte).with_cte(files_cte) | ||
| staging_tree_files = ( | ||
| staging_tree_files.exclude( | ||
| Exists(files_cte.queryset().filter(checksum=OuterRef("checksum"))) | ||
| ) | ||
| .values("checksum") | ||
| .distinct() | ||
| ) | ||
|
|
@@ -412,11 +412,22 @@ def get_user_active_trees(self): | |
| def get_user_active_files(self): | ||
| cte = With(self.get_user_active_trees().distinct()) | ||
|
|
||
| return ( | ||
| cte.join(self.files.get_queryset(), contentnode__tree_id=cte.col.tree_id) | ||
| .with_cte(cte) | ||
| .values("checksum") | ||
| .distinct() | ||
| files_qs = cte.join( | ||
| self.files.get_queryset(), contentnode__tree_id=cte.col.tree_id | ||
| ).with_cte(cte) | ||
|
|
||
| files_qs = self._filter_storage_billable_files(files_qs) | ||
|
|
||
| return files_qs.values("checksum").distinct() | ||
|
|
||
| def _filter_storage_billable_files(self, queryset): | ||
| """ | ||
| Perseus exports would not be included in storage calculations. | ||
| """ | ||
| if queryset is None: | ||
| return queryset | ||
| return queryset.exclude(file_format_id__isnull=True).exclude( | ||
| file_format_id=file_formats.PERSEUS | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not an immediate concern, but just a heads that when QTI assessments are more broadly available, and we are generating QTI ZIP files, then we may need to filter these too (and it would need to be on the format preset, rather than the file format id, because the format id would be 'zip'!) |
||
| ) | ||
|
|
||
| def get_space_used(self, active_files=None): | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
As we looked at together, the combination of
self.files.get_queryset()and the tree filtering is blowing up the performance of the query. Breaking this down into smaller blocks makes it more performant and allows for the additional filtering you're adding. I think something like this might work:See if you can apply some of the same ideas to the more complex
check_channel_spacemethod too.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.
The main
files_qsmight also need.with_cte(cte)too. I'm a bit unsure