-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
813 - Persist portal metadata computed file and upload after creation #825
Merged
nozomione
merged 96 commits into
feature/portal-metadata-command
from
nozomione/813-persist-portal-metadata-computed-file-data
Sep 6, 2024
Merged
Changes from all commits
Commits
Show all changes
96 commits
Select commit
Hold shift + click to select a range
5f38079
create the management command file for the portal-wide metadata and r…
nozomione 62bff10
create the test file for the portal-wide matadata management command …
nozomione f4df115
(edit) rename the setup_database method to load_test_data, remove 'pr…
nozomione 7118063
Merge remote-tracking branch 'origin/feature/portal-metadata-command'…
nozomione 4b07459
add ComputedFile::get_portal_metadata_file method and readme_file.get…
nozomione 3d9f6af
(edit) call ComputedFile::get_portal_metadata_file in create-portal-m…
nozomione 3538049
(edit) add the portal metadata file generation workflow to ComputedFi…
nozomione 301cab9
(edit) adjust the create portal metadata command and its test (add te…
nozomione 6250be9
Merge remote-tracking branch 'origin/feature/portal-metadata-command'…
nozomione 72e8664
(edit) check against queryset objects count rather than IDs for proje…
nozomione fc3b731
(minor) fix a typo and remove comments
nozomione 468c52d
Merge branch 'nozomione/797-scaffolding-management-command-file-1' in…
nozomione 836a8d9
Merge branch 'origin/feature/portal-metadata-command' into nozomione/…
nozomione d5ce758
add common.GENERATED_PORTAL_METADATA_DOWNLOAD_CONFIG for the portal m…
nozomione 7b416aa
(edit) edit readme_file.get_file_contents temporarily(and remove get_…
nozomione 37e29d8
(edit) check the portal_metadata_only key to swap config and template…
nozomione 9474b77
(edit) add a type hint for the queryset parameter
nozomione ef38860
(merge) resolve conflict and merge the latest dev branch updates to n…
nozomione 690aa4b
(edit) store the portal metadata computed file to a local variable co…
nozomione 2a0f7ef
Merge branch 'nozomione/797-generate-readme-file-zip-2' into nozomion…
nozomione f26e0d7
(edit) add the static method ComputedFile::get_local_portal_metadata_…
nozomione d001b6a
Merge 'dev' branch into nozomione/813-persist-portal-metadata-compute…
nozomione 03a3791
(edit) add save/upload logic to the create portal metadata management…
nozomione 428b9f8
(fix) fixed the readme output
nozomione 5283d28
Merge 'dev' branch into nozomione/797-generate-readme-file-zip-2
nozomione c65e59e
Merge branch 'nozomione/797-generate-readme-file-zip-2' into nozomion…
nozomione 1628d3e
Merge branch 'nozomione/797-generate-metadata-file-zip-3' into nozomi…
nozomione 9024959
(merge) resolve conflict and merge the tracking feature/portal-metada…
nozomione 3435b18
(edit) adjust readme_file.get_file_contents using new readme template…
nozomione 9a2728c
(edit) remove the test_zip_file and test_readme_file methods and asse…
nozomione d379f7e
(merge) resolve conflict and merge 'nozomione/797-generate-readme-fil…
nozomione f5104a2
(bug) remove the extra context manager for readme check
nozomione fb2533d
Merge 'nozomione/797-generate-readme-file-zip-2' into nozomione/797-g…
nozomione bfde9f6
(edit) remove the test_metadata_file method and directly assert the m…
nozomione b2d26cb
(minor) move comments and variable for zip assertion inside the conte…
nozomione 8ddd3b1
Merge 'nozomione/797-generate-readme-file-zip-2' into nozomione/797-g…
nozomione 4aeb76e
(edit) use the csv module's DictReader for the metadata.tsv file asse…
nozomione 31ff9af
Merge nozomione/797-generate-metadata-file-zip-3 into nozomione/813-p…
nozomione 54a203d
(edit) remove TODO comment and move the body of test_computed_file in…
nozomione 4c68764
(minor) remove the LOCAL_ZIP_FILE_PATH variable (no need - since the …
nozomione 011abdf
(typo) fix a typo
nozomione 20564c4
Merge branch 'dev' into nozomione/797-generate-readme-file-zip-2
nozomione a7e56b5
Merge branch 'feature/portal-metadata-command' into nozomione/797-gen…
nozomione 9c703c8
Merge remote-tracking branch 'origin/nozomione/797-generate-readme-fi…
nozomione 0d12432
(minor) remove a comment in computed_file, instead it wil be included…
nozomione f3ce338
Merge branch 'nozomione/797-generate-metadata-file-zip-3' into nozomi…
nozomione 3f0ccd5
Merge branch 'feature/portal-metadata-command' into nozomione/797-gen…
nozomione 7213be4
Merge branch 'nozomione/797-generate-readme-file-zip-2' into nozomion…
nozomione 7ecd55c
Merge branch 'nozomione/797-generate-metadata-file-zip-3' into nozomi…
nozomione 802080e
(edit) add an arg for upload_s3 and define the constans for args' def…
nozomione 0200be7
(rename) append the suffix '_FILE' to the constants README and METADATA
nozomione 1f5e074
(rename) rename common.GENERATED_SAMPLE_DOWNLOAD_CONFIGS (to plural) …
nozomione ed9e0e4
(edit) refactor download_config handling for portal-metadata (revert …
nozomione 017b992
(clean up) remove the constant added previously(no longer used)
nozomione 9bd173f
Merge branch 'nozomione/797-generate-readme-file-zip-2' into nozomion…
nozomione 5d81c0e
(edit) use list literals to add a check that matches the project down…
nozomione 3cfca8a
Merge branch 'nozomione/797-generate-readme-file-zip-2' into nozomion…
nozomione 02cdde6
add a new static method ComputedFile::get_local_file_path (remove Com…
nozomione cbc25eb
Merge 'nozomione/797-generate-metadata-file-zip-3' branch into nozomi…
nozomione 0e5cf01
Merge branch 'dev' into nozomione/813-persist-portal-metadata-compute…
nozomione 03b7b40
(migration) merge migration files to resolve multiple leaf nodes
nozomione ffcc45e
(edit) use s3.upload_output_file (remove computed_file.upload_s3_file…
nozomione 0c7b5ae
(minor) remove the constant 'ENCODING' to match the implementation of…
nozomione e7ba46a
(TODO comment) add a TODO comment to indicate that once PR #839 is me…
nozomione 189573f
(edit) add the command 'configure_aws_cli' to resolve the duplicated …
nozomione 970d67e
(TENP) temporaily skip isort to modify the import order of the manage…
nozomione 0a8b524
(edit) add mock for s3.upload_output_file used in create_portal_metadata
nozomione dc12498
(migration) undo the merged migration files
nozomione b910454
(fix) run pre-commit and migrate
nozomione d87b7d5
Merge branch 'feature/portal-metadata-command' into nozomione/797-gen…
nozomione 2c66417
(adjust) rollblack previous migration and re-migrate the portal_metad…
nozomione 8610545
Merge branch 'nozomione/797-generate-readme-file-zip-2' into nozomion…
nozomione 631cef9
Merge branch 'nozomione/797-generate-metadata-file-zip-3' into nozomi…
nozomione cfe68bf
(mionr) re-locate the logger message and fix typos in create_portal_m…
nozomione 450d11a
(minor) make another revision of the comments
nozomione baa659c
(remove TEMP) remove temporaily added codeblocks
nozomione 2b77b7a
(edit) access kwargs props using square brackets instead of the get m…
nozomione 48ab892
(edit) use the get method to give the default value in if condition t…
nozomione 0258446
(minor) add a check for adding logging handler to make sure no duplic…
nozomione 182ab63
(minor) add a comment and remove the handler var (no needed)
nozomione b692665
(minor) add a comment for the handler check
nozomione 964fa78
(edit) rename GENERATED_PROJECT_DOWNLOAD_CONFIG to GENERATED_PROJECT_…
nozomione f2574ef
Merge branch 'nozomione/797-generate-readme-file-zip-2' into nozomion…
nozomione 651d753
(edit) use the config.logging.get_and_configure_logger for the log me…
nozomione 52c6daf
Merge 'nozomione/797-generate-metadata-file-zip-3' branch into nozomi…
nozomione 9454372
Merge 'feature/portal-metadata-command' branch into nozomione/797-gen…
nozomione 68053c9
(edit) add Iterable type hint instead of Queryset in readme_file.get_…
nozomione e0cf99f
Merge 'nozomione/797-generate-readme-file-zip-2)' into nozomione/797-…
nozomione afadbac
Merge 'nozomione/797-generate-metadata-file-zip-3' into nozomione/813…
nozomione 609dc06
(edit) pass the argument 'computed_file.s3_bucket' to 's3.upload_outo…
nozomione d1f8a25
(edit) clean up output data regardless of computed file existence and…
nozomione e652a76
Merge branch 'feature/portal-metadata-command' into nozomione/813-per…
nozomione 0300b9f
(fix) adjust the computed file byte size in the test_create_portal)me…
nozomione cd59c14
(edit) use walrus operator for computed_file assignment and save comp…
nozomione daccc79
(edit) add assertEqualWithVariance (which checks computed_file file s…
nozomione 75ed5b3
(edit) remove data type checks for the field values and instad, perfo…
nozomione 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 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 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 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 |
---|---|---|
@@ -1,6 +1,8 @@ | ||
import csv | ||
import shutil | ||
from io import TextIOWrapper | ||
from typing import Dict | ||
from unittest.mock import patch | ||
from zipfile import ZipFile | ||
|
||
from django.conf import settings | ||
|
@@ -29,9 +31,6 @@ def tearDownClass(cls): | |
super().tearDownClass() | ||
shutil.rmtree(common.OUTPUT_DATA_PATH, ignore_errors=True) | ||
|
||
def assertProjectReadmeContains(self, text, zip_file): | ||
self.assertIn(text, zip_file.read("README.md").decode("utf-8")) | ||
|
||
def load_test_data(self): | ||
# Expected object counts | ||
PROJECTS_COUNT = 3 | ||
|
@@ -54,31 +53,71 @@ def load_test_data(self): | |
self.assertEqual(Sample.objects.all().count(), SAMPLES_COUNT) | ||
self.assertEqual(Library.objects.all().count(), LIBRARIES_COUNT) | ||
|
||
def test_create_portal_metadata(self): | ||
# TODO: After PR #839 is merged into dev, add readme file format testing | ||
def assertProjectReadmeContains(self, text, zip_file): | ||
self.assertIn(text, zip_file.read(README_FILE).decode("utf-8")) | ||
Comment on lines
+56
to
+58
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. Good point, we should draft the issue to do this instead of just having the comment. 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. I've gone ahead and filed the issue here #869 👍! |
||
|
||
def assertFields(self, computed_file, expected_fields: Dict): | ||
for expected_key, expected_value in expected_fields.items(): | ||
actual_value = getattr(computed_file, expected_key) | ||
message = f"Expected {expected_value}, received {actual_value} on '{expected_key}'" | ||
self.assertEqual(actual_value, expected_value, message) | ||
|
||
def assertEqualWithVariance(self, value, expected, variance=50): | ||
# Make sure the given value is within the range of expected bounds | ||
message = f"{value} is out of range" | ||
self.assertGreaterEqual(value, expected - variance, message) | ||
self.assertLessEqual(value, expected + variance, message) | ||
|
||
@patch("scpca_portal.management.commands.create_portal_metadata.s3.upload_output_file") | ||
def test_create_portal_metadata(self, mock_upload_output_file): | ||
# Set up the database for test | ||
self.load_test_data() | ||
self.processor.create_portal_metadata(clean_up_output_data=False) | ||
# Create the portal metadata computed file | ||
self.processor.create_portal_metadata(clean_up_output_data=False, update_s3=True) | ||
|
||
# Test the computed file | ||
computed_files = ComputedFile.objects.filter(portal_metadata_only=True) | ||
# Make sure the computed file is created and singular | ||
self.assertEqual(computed_files.count(), 1) | ||
computed_file = computed_files.first() | ||
# Make sure the computed file size is as expected range | ||
self.assertEqualWithVariance(computed_file.size_in_bytes, 8430) | ||
# Make sure all fields match the download configuration values | ||
download_config = { | ||
"modality": None, | ||
"format": None, | ||
"includes_merged": False, | ||
"metadata_only": True, | ||
"portal_metadata_only": True, | ||
} | ||
self.assertFields(computed_file, download_config) | ||
# Make sure mock_upload_output_file called once | ||
mock_upload_output_file.assert_called_once_with( | ||
computed_file.s3_key, computed_file.s3_bucket | ||
) | ||
|
||
# Test the content of the generated zip file | ||
zip_file_path = ComputedFile.get_local_file_path( | ||
common.GENERATED_PORTAL_METADATA_DOWNLOAD_CONFIG | ||
) | ||
with ZipFile(zip_file_path) as zip: | ||
# Test the content of the generated zip file | ||
with ZipFile(zip_file_path) as zip_file: | ||
# There are 2 file: | ||
# ├── README.md | ||
# |── metadata.tsv | ||
expected_file_count = 2 | ||
# Make sure the zip has the exact number of expected files | ||
files = set(zip.namelist()) | ||
files = set(zip_file.namelist()) | ||
self.assertEqual(len(files), expected_file_count) | ||
self.assertIn(README_FILE, files) | ||
self.assertIn(METADATA_FILE, files) | ||
# README.md | ||
expected_text = ( | ||
"This download includes associated metadata for samples from all projects" | ||
) | ||
self.assertProjectReadmeContains(expected_text, zip) | ||
self.assertProjectReadmeContains(expected_text, zip_file) | ||
# metadata.tsv | ||
with zip.open(METADATA_FILE) as metadata_file: | ||
with zip_file.open(METADATA_FILE) as metadata_file: | ||
csv_reader = csv.DictReader( | ||
TextIOWrapper(metadata_file, "utf-8"), | ||
delimiter=common.TAB, | ||
|
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.
Let's move updating above saving it to the DB, so that if it fails on upload we exit and the file is unavailable.