915 - Add generate computed file command #936

avrohomgottlieb · 2024-10-15T17:34:27Z

Issue Number

Closes #915 and #841

Purpose/Implementation Notes

This PR makes 3 additions

Adds a loader::generate_computed_file method
Adds a generate_computed_file management command, which will be used as the entrypoint with Batch jobs
Adds a dispatch_to_batch management command, responsible to iterating over all permutations of projects, samples and downloads_configs which need computed files generated for them

Types of changes

What types of changes does your code introduce?

Bugfix (non-breaking change which fixes an issue)
Refactor (addresses code organization and design mentioned in corresponding issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Functional tests

List out the functional tests you've completed to verify your changes work locally.

Checklist

Lint and unit tests pass locally with my changes

Screenshots

N/A

… mistakes

…ted_file_callback and loader::_create_computed_file

davidsmejia · 2024-10-15T17:35:36Z

api/scpca_portal/loader.py

+
+def _create_computed_file_callback(future, *, update_s3: bool, clean_up_output_data: bool) -> None:
+    """
+    Wrap multiprocessing logic by grabbing computed file future and uploading it tohe s3.


davidsmejia · 2024-10-15T17:48:59Z

api/scpca_portal/management/commands/generate_computed_file.py

+        self,
+        project_id: str,
+        sample_id: str,
+        download_config: Dict,


This should be a string that references a dict in common.py

davidsmejia · 2024-10-15T18:09:43Z

api/scpca_portal/loader.py

+    if project:
+        computed_file = ComputedFile.get_project_file(
+            project, download_config, project.get_output_file_name(download_config)
+        )
+        _create_computed_file(computed_file, update_s3, clean_up_output_data=False)
+    elif sample:
+        computed_file = ComputedFile.get_sample_file(
+            sample,
+            download_config,
+            sample.get_output_file_name(download_config),
+            Lock(),  # this should be removed when CF::get_sample_file is refactored
+        )
+        _create_computed_file(computed_file, update_s3, clean_up_output_data=False)
+        sample.project.update_downloadable_sample_count()


Suggested change

if project:

computed_file = ComputedFile.get_project_file(

project, download_config, project.get_output_file_name(download_config)

)

_create_computed_file(computed_file, update_s3, clean_up_output_data=False)

elif sample:

computed_file = ComputedFile.get_sample_file(

sample,

download_config,

sample.get_output_file_name(download_config),

Lock(), # this should be removed when CF::get_sample_file is refactored

)

_create_computed_file(computed_file, update_s3, clean_up_output_data=False)

sample.project.update_downloadable_sample_count()

if project and computed_file := ComputedFile.get_project_file(project, download_config):

_create_computed_file(computed_file, update_s3, clean_up_output_data=False)

if sample and computed_file := ComputedFile.get_sample_file(sample, download_config):

_create_computed_file(computed_file, update_s3, clean_up_output_data=False)

sample.project.update_downloadable_sample_count()

I think you can make Lock() the default value for a lock in get_sample_file. Otherwise rewriting it this way makes it a bit clearer.

…iles-command

davidsmejia

Looks mostly correct, just some bugs / changes that we should discuss again before implementing.

davidsmejia · 2024-10-25T19:47:00Z

api/scpca_portal/management/commands/dispatch_to_batch.py

+        projects = (
+            Project.objects.filter(project_computed_files__is_null=True)
+            if not project_id
+            else [Project.objects.filter(scpca_id=project_id).first()]


Suggested change

else [Project.objects.filter(scpca_id=project_id).first()]

else Project.objects.filter(scpca_id=project_id)

davidsmejia · 2024-10-25T19:52:19Z

api/scpca_portal/management/commands/generate_computed_file.py

+            if download_config_name not in common.PROJECT_DOWNLOAD_CONFIGS.keys():
+                logger.error(f"{download_config_name} is not a valid project download config name.")
+                logger.info(
+                    f"Here are the correct project download_config names: "


Suggested change

f"Here are the correct project download_config names: "

f"Here are valid download_config_name values for projects: "

davidsmejia · 2024-10-25T19:52:48Z

api/scpca_portal/management/commands/generate_computed_file.py

+            if download_config_name not in common.SAMPLE_DOWNLOAD_CONFIGS.keys():
+                logger.error(f"{download_config_name} is not a valid sample download config name.")
+                logger.info(
+                    f"Here are the correct sample download_config names: "


Suggested change

f"Here are the correct sample download_config names: "

f"Here are valid download_config_name values for samples: "

davidsmejia · 2024-10-25T19:53:41Z

api/scpca_portal/management/commands/generate_computed_file.py

+                    f"Here are the correct sample download_config names: "
+                    f"{common.SAMPLE_DOWNLOAD_CONFIGS.keys()}"
+                )
+            download_config = common.PROJECT_DOWNLOAD_CONFIGS[download_config_name]


Suggested change

download_config = common.PROJECT_DOWNLOAD_CONFIGS[download_config_name]

download_config = common.SAMPLE_DOWNLOAD_CONFIGS[download_config_name]

davidsmejia · 2024-10-25T19:56:27Z

api/scpca_portal/models/project.py

@@ -205,9 +205,9 @@ def get_output_file_name(self, download_config: Dict) -> str:
    def get_computed_file(self, download_config: Dict) -> ComputedFile:
        "Return the project computed file that matches the passed download_config."
        if download_config["metadata_only"]:
-            return self.computed_files.filter(metadata_only=True).first()
+            return self.project_computed_files.filter(metadata_only=True).first()


Suggested change

return self.project_computed_files.filter(metadata_only=True).first()

return self.computed_files.filter(metadata_only=True).first()

Let's keep using the property for now.

davidsmejia · 2024-10-25T19:56:38Z

api/scpca_portal/models/project.py


-        return self.computed_files.filter(
+        return self.project_computed_files.filter(


Suggested change

return self.project_computed_files.filter(

return self.computed_files.filter(

davidsmejia · 2024-10-25T19:57:05Z

api/scpca_portal/models/sample.py

@@ -123,7 +123,7 @@ def get_metadata(self) -> Dict:

    def get_computed_file(self, download_config: Dict) -> ComputedFile:
        "Return the sample computed file that matches the passed download_config."
-        return self.computed_files.filter(
+        return self.sample_computed_files.filter(


Suggested change

return self.sample_computed_files.filter(

return self.computed_files.filter(

…ethods

avrohomgottlieb added 8 commits October 14, 2024 15:09

fix hidden computed_file query bug in Project::purge_computed_files

92a7849

add Sample::purge_computed_files method

82ce51e

add ComputedFile::purge, fix a few Project and Sample CF related name…

b578d27

… mistakes

add break up loader::_create_computed_file into loader::_create_compu…

b9d8476

…ted_file_callback and loader::_create_computed_file

add loader::generate_computed_file function

8df351d

convert all args to kwargs in loader::generate_computed_file

19bd10e

add generate_computed_file management command

77ae571

add validation to generate_computed_file command

6a8c4f5

vercel bot deployed to Preview October 15, 2024 17:34 View deployment

davidsmejia reviewed Oct 15, 2024

View reviewed changes

avrohomgottlieb changed the base branch from dev to feature/batch October 16, 2024 13:30

avrohomgottlieb added 5 commits October 16, 2024 11:03

remove computed_file_name parameter from CF::get_project|sample_file

0bd2539

improve readability of loader::generate_computed_file

4a30d4e

remove return early statements, clean up logging statements

f2c04b3

Merge branch 'feature/batch' into avrohom/915-add-generate-computed-f…

99a1175

…iles-command

add dispatch_to_batch command

a2bf77c

avrohomgottlieb marked this pull request as ready for review October 16, 2024 16:18

vercel bot deployed to Preview October 16, 2024 16:20 View deployment

avrohomgottlieb linked an issue Oct 16, 2024 that may be closed by this pull request

Create dispatch_computed_file_generation command #841

Closed

davidsmejia reviewed Oct 25, 2024

View reviewed changes

avrohomgottlieb added 3 commits October 28, 2024 09:44

improve logging statements, fix download config reference bug

d12761f

improve code quality in projects query

5b10871

enforce usage of computed_file property in project and sample model m…

b8bfdbc

…ethods

vercel bot deployed to Preview October 28, 2024 13:52 View deployment

avrohomgottlieb requested a review from davidsmejia October 28, 2024 19:11

avrohomgottlieb changed the title ~~915 - Add generate computed files command~~ 915 - Add generate computed file command Oct 29, 2024

davidsmejia approved these changes Oct 29, 2024

View reviewed changes

avrohomgottlieb merged commit 5ab62f6 into feature/batch Oct 29, 2024
5 checks passed

avrohomgottlieb deleted the avrohom/915-add-generate-computed-files-command branch October 29, 2024 19:38

avrohomgottlieb mentioned this pull request Nov 1, 2024

Add generate_computed_file Command #915

Closed

avrohomgottlieb mentioned this pull request Nov 1, 2024

Create dispatch_computed_file_generation command #841

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

915 - Add generate computed file command #936

915 - Add generate computed file command #936

avrohomgottlieb commented Oct 15, 2024 •

edited

Loading

davidsmejia Oct 15, 2024

davidsmejia Oct 15, 2024

davidsmejia Oct 15, 2024

davidsmejia left a comment

davidsmejia Oct 25, 2024

davidsmejia Oct 25, 2024

davidsmejia Oct 25, 2024

davidsmejia Oct 25, 2024

davidsmejia Oct 25, 2024

davidsmejia Oct 25, 2024

davidsmejia Oct 25, 2024

	else [Project.objects.filter(scpca_id=project_id).first()]
	else Project.objects.filter(scpca_id=project_id)

	f"Here are the correct project download_config names: "
	f"Here are valid download_config_name values for projects: "

	f"Here are the correct sample download_config names: "
	f"Here are valid download_config_name values for samples: "

	download_config = common.PROJECT_DOWNLOAD_CONFIGS[download_config_name]
	download_config = common.SAMPLE_DOWNLOAD_CONFIGS[download_config_name]

	return self.project_computed_files.filter(metadata_only=True).first()
	return self.computed_files.filter(metadata_only=True).first()


		return self.computed_files.filter(
		return self.project_computed_files.filter(

	return self.sample_computed_files.filter(
	return self.computed_files.filter(

915 - Add generate computed file command #936

915 - Add generate computed file command #936

Conversation

avrohomgottlieb commented Oct 15, 2024 • edited Loading

Issue Number

Purpose/Implementation Notes

Types of changes

Functional tests

Checklist

Screenshots

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidsmejia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avrohomgottlieb commented Oct 15, 2024 •

edited

Loading