Expose 'submission_date' and 'update_date' (#2562, #3310) #3026

dsotirho-ucsc · 2021-04-29T17:00:45Z

#2562
#3310

PR title references issue
Title of main commit references issue
PR is linked to Zenhub issue

Author (reindex)

Added r tag to commit title _{or this PR does not require reindexing}
Added reindex label to PR _{or this PR does not require reindexing}

Author (freebies & chains)

Freebies are blocked on this PR _{or there are no freebies in this PR}
Freebies are referenced in commit titles _{or there are no freebies in this PR}
This PR is blocked by previous PR in the chain _{or this PR is not chained to another PR}
Added chain label to the blocking PR _{or this PR is not chained to another PR}

Author (upgrading)

Documented upgrading of deployments in UPGRADING.rst _{or this PR does not require upgrading}
Added u tag to commit title _{or this PR does not require upgrading}
Added upgrade label to PR _{or this PR does not require upgrading}

Author (requirements, before every review)

Ran make requirements_update _{or this PR leaves requirements*.txt, common.mk and Makefile untouched}
Added R tag to commit title _{or this PR leaves requirements*.txt untouched}
Added reqs label to PR _{or this PR leaves requirements*.txt untouched}

Author (before every review)

make integration_test passes in personal deployment _{or this PR does not touch functionality that could break the IT}
Rebased branch on develop, squashed old fixups

Primary reviewer (after approval)

Commented on demo expectations _{or labelled PR as no demo}
Decided if PR can be labeled no sandbox
PR title is appropriate as title of merge commit
Moved ticket to Approved column
Assigned PR to an operator

Operator (before pushing merge the commit)

Operator (after pushing the merge commit)

Moved freebies to Merged column _{or there are no freebies in this PR}
Shortened the PR chain _{or this PR is not the base of another PR}
Verified that N reviews labelling is accurate
Pushed merge commit to Gitlab _{or this changes can be pushed later, together with another PR}
Deleted PR branch from Github and Gitlab

Operator (reindex)

Started reindex in dev _{or this PR does not require reindexing or does not target dev}
Checked for failures in dev _{or this PR does not require reindexing or does not target dev}
Started reindex in prod _{or this PR does not require reindexing or does not target prod}
Checked for failures in prod _{or this PR does not require reindexing or does not target prod}

Operator

Unassigned PR

codecov · 2021-04-29T18:55:49Z

Codecov Report

Merging #3026 (6a8498d) into develop (4520816) will increase coverage by 0.13%.
The diff coverage is 97.95%.

❗ Current head 6a8498d differs from pull request most recent head 54f948d. Consider uploading reports for the commit 54f948d to get more accurate results

@@             Coverage Diff             @@
##           develop    #3026      +/-   ##
===========================================
+ Coverage    82.36%   82.50%   +0.13%     
===========================================
  Files          123      123              
  Lines        13998    14035      +37     
===========================================
+ Hits         11529    11579      +50     
+ Misses        2469     2456      -13

Impacted Files	Coverage Δ
src/azul/__init__.py	`81.17% <ø> (ø)`
test/indexer/test_hca_indexer.py	`99.30% <ø> (ø)`
test/service/test_repository_projects.py	`100.00% <ø> (ø)`
test/service/test_manifest.py	`99.56% <75.00%> (-0.01%)`	⬇️
src/azul/plugins/metadata/hca/aggregate.py	`97.77% <94.11%> (+0.05%)`	⬆️
src/azul/indexer/aggregate.py	`90.28% <100.00%> (+2.71%)`	⬆️
src/azul/indexer/document.py	`95.90% <100.00%> (+0.17%)`	⬆️
src/azul/plugins/metadata/hca/transform.py	`98.78% <100.00%> (-0.15%)`	⬇️
src/azul/plugins/repository/tdr/__init__.py	`90.63% <100.00%> (+0.03%)`	⬆️
src/azul/service/avro_pfb.py	`96.18% <100.00%> (+0.42%)`	⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4520816...54f948d. Read the comment docs.

coveralls · 2021-04-29T18:56:33Z

Coverage increased (+0.1%) to 82.716% when pulling 54f948d on issues/danielsotirhos/2562-expose-projects-submission-date-and-update-date into 4520816 on develop.

hannes-ucsc

So far so good. Could you spend two hours investigating what it would entail to have these two properties exposed for every inner entity? This would allow us to expose not only when the project itself was updated but also entities that contribute to it. I think this would imply that we need to aggregate over the properties, I think using min for submission_date and max for update_date.

hannes-ucsc

Whenever you copy and paste the same code multiple times, an alarm bell needs to go off.

hannes-ucsc · 2021-05-08T00:56:28Z

src/azul/plugins/metadata/hca/aggregate.py

+        elif field == 'update_date':
+            return MaxAccumulator()
+        else:
+            return SetAccumulator(max_size=10)


Why 10 and not 100?

hannes-ucsc · 2021-05-08T00:59:08Z

src/azul/plugins/metadata/hca/aggregate.py

+        elif field == 'submission_date':
+            return MinAccumulator()
+        elif field == 'update_date':
+            return MaxAccumulator()
        else:
            return SetAccumulator(max_size=100)


This is all painfully duplicative. We should adjust the default implementation so we can write

Suggested change

elif field == 'submission_date':

return MinAccumulator()

elif field == 'update_date':

return MaxAccumulator()

else:

return SetAccumulator(max_size=100)

else:

return super()._get_accumulator()

without affecting the semantics.

hannes-ucsc · 2021-05-08T01:01:19Z

src/azul/plugins/metadata/hca/aggregate.py

        else:
            return None


This is different to the other implementations of this method. We should create a new _get_default_accumulator() method in the base class and override that here to return None instead of SetAccumulator(max_size=100).

hannes-ucsc · 2021-05-08T01:05:22Z

src/azul/plugins/metadata/hca/transform.py

            'document_id': str(cell_line.document_id),
            'biomaterial_id': cell_line.biomaterial_id,


Suggested change

'document_id': str(cell_line.document_id),

'biomaterial_id': cell_line.biomaterial_id,

**_biomaterial(cell_line),

_biomaterial calls _entity and adds and adds an entry for biomaterial_id

Similar structure for the _foo_types methods.

hannes-ucsc · 2021-05-08T01:05:35Z

src/azul/plugins/metadata/hca/transform.py

+            'submission_date': cell_line.submission_date,
+            'update_date': cell_line.update_date,


Suggested change

'submission_date': cell_line.submission_date,

'update_date': cell_line.update_date,

hannes-ucsc · 2021-05-08T01:07:14Z

src/azul/plugins/metadata/hca/transform.py

@@ -782,74 +815,96 @@ def _related_file(self, file: api.File) -> MutableJSON:
            'uuid': file.manifest_entry.uuid,
            'drs_path': self.bundle.drs_path(file.manifest_entry.json),
            'version': file.manifest_entry.version,
+            'submission_date': file.submission_date,
+            'update_date': file.update_date,
        }

    @classmethod
    def _analysis_protocol_types(cls) -> FieldTypes:
        return {
            'document_id': null_str,


Suggested change

'document_id': null_str,

**_entity(file),

hannes-ucsc · 2021-05-08T01:07:30Z

src/azul/plugins/metadata/hca/transform.py

+            'submission_date': null_str,
+            'update_date': null_str,


Suggested change

'submission_date': null_str,

'update_date': null_str,

hannes-ucsc · 2021-05-08T01:10:02Z

src/azul/service/hca_response_v5.py

+                    'submissionDate': p['submission_date'],
+                    'updateDate': p['update_date'],


Same refactoring here.

hannes-ucsc

Commit title Add "Assert Messaging" to Contributing document is bad. Neither the section name nor the file name matches.

This PR is connected to #3310 but there is no commit referencing that issue. The ticket references in commit titles are EXTREMELY important because when done right, the commit shows up in the ticket which is INVALUABLE when trying to make sense of things later.

I think we may want to commit the workaround for the Python bug as a separate commit.

hannes-ucsc · 2021-08-19T17:00:00Z

CONTRIBUTING.rst

@@ -478,6 +478,38 @@ Static methods
  higher order concerns than the one about whether a method's body currently
  references self or not.

+Error Messages


Suggested change

Error Messages

Error messages

hannes-ucsc · 2021-08-19T17:00:26Z

CONTRIBUTING.rst

+Error Messages
+----------------


Suggested change

Error Messages

----------------

Error Messages

--------------

hannes-ucsc

I renamed one of the tickets. Could you update the commit title accordingly?

No bump.

jessebrennan · 2021-08-24T20:37:12Z

@danielsotirhos the PR title only referenced on of the commits addressed here. I've updated it, but this is how I messed up the merge commit on develop

jessebrennan · 2021-08-26T17:24:26Z

Failures on dev matched expected tickets:

There were no unexpected failures.

github-actions bot added the orange [process] Done by the Azul team label Apr 29, 2021

dsotirho-ucsc added the reindex:dev [process] PR requires reindexing dev label Apr 29, 2021

dsotirho-ucsc force-pushed the issues/danielsotirhos/2562-expose-projects-submission-date-and-update-date branch 3 times, most recently from f802ed7 to d8c6459 Compare April 29, 2021 18:17

dsotirho-ucsc force-pushed the issues/danielsotirhos/2562-expose-projects-submission-date-and-update-date branch from d8c6459 to e374041 Compare April 29, 2021 22:06

dsotirho-ucsc requested a review from amarjandu April 29, 2021 23:48

dsotirho-ucsc assigned amarjandu Apr 29, 2021

amarjandu approved these changes Apr 30, 2021

View reviewed changes

dsotirho-ucsc force-pushed the issues/danielsotirhos/2562-expose-projects-submission-date-and-update-date branch from e374041 to 3cf1af4 Compare April 30, 2021 20:34

dsotirho-ucsc unassigned amarjandu Apr 30, 2021

dsotirho-ucsc requested a review from hannes-ucsc April 30, 2021 21:00

dsotirho-ucsc assigned hannes-ucsc Apr 30, 2021

hannes-ucsc reviewed May 3, 2021

View reviewed changes

hannes-ucsc added the 0 reviews [process] Lead didn't request any changes label May 3, 2021

hannes-ucsc removed their assignment May 3, 2021

dsotirho-ucsc force-pushed the issues/danielsotirhos/2562-expose-projects-submission-date-and-update-date branch 2 times, most recently from adef2d9 to 2e8b35f Compare May 5, 2021 18:04

dsotirho-ucsc requested a review from hannes-ucsc May 6, 2021 03:28

dsotirho-ucsc assigned hannes-ucsc May 6, 2021

hannes-ucsc requested changes May 8, 2021

View reviewed changes

hannes-ucsc added 1 review [process] Lead requested changes once and removed 0 reviews [process] Lead didn't request any changes labels May 8, 2021

hannes-ucsc removed their assignment May 8, 2021

dsotirho-ucsc force-pushed the issues/danielsotirhos/2562-expose-projects-submission-date-and-update-date branch 3 times, most recently from 7a3a01a to d6f5901 Compare May 12, 2021 17:15

dsotirho-ucsc requested a review from hannes-ucsc May 12, 2021 18:02

dsotirho-ucsc requested a review from hannes-ucsc August 19, 2021 16:53

dsotirho-ucsc assigned hannes-ucsc Aug 19, 2021

hannes-ucsc requested changes Aug 19, 2021

View reviewed changes

hannes-ucsc removed their assignment Aug 19, 2021

dsotirho-ucsc force-pushed the issues/danielsotirhos/2562-expose-projects-submission-date-and-update-date branch 2 times, most recently from d408410 to 6a8498d Compare August 19, 2021 22:53

dsotirho-ucsc requested a review from hannes-ucsc August 19, 2021 23:18

dsotirho-ucsc assigned hannes-ucsc Aug 19, 2021

hannes-ucsc requested changes Aug 20, 2021

View reviewed changes

hannes-ucsc removed their assignment Aug 20, 2021

dsotirho-ucsc force-pushed the issues/danielsotirhos/2562-expose-projects-submission-date-and-update-date branch from 6a8498d to 99785e5 Compare August 20, 2021 05:54

dsotirho-ucsc requested a review from hannes-ucsc August 20, 2021 15:40

dsotirho-ucsc assigned hannes-ucsc Aug 20, 2021

hannes-ucsc approved these changes Aug 20, 2021

View reviewed changes

hannes-ucsc assigned jessebrennan and unassigned hannes-ucsc Aug 20, 2021

jessebrennan changed the title ~~Expose 'submission_date' and 'update_date' (#2562)~~ [r] Expose 'submission_date' and 'update_date' (#2562) Aug 24, 2021

jessebrennan changed the title ~~[r] Expose 'submission_date' and 'update_date' (#2562)~~ Expose 'submission_date' and 'update_date' (#2562) Aug 24, 2021

dsotirho-ucsc added 3 commits August 23, 2021 18:22

[r] Expose 'submission_date' and 'update_date' (#2562)

06185f3

Fix: strftime fails to zero pad year < 1000 on AWS Lambda (#3310)

6c1cf50

Add "Error messages" section to CONTRIBUTING.rst

54f948d

jessebrennan force-pushed the issues/danielsotirhos/2562-expose-projects-submission-date-and-update-date branch from 99785e5 to 54f948d Compare August 24, 2021 01:22

jessebrennan added the sandbox [process] Resolution is being verified in sandbox deployment label Aug 24, 2021

jessebrennan removed their assignment Aug 24, 2021

jessebrennan merged this pull request into develop Aug 24, 2021

jessebrennan changed the title ~~Expose 'submission_date' and 'update_date' (#2562)~~ Expose 'submission_date' and 'update_date' (#2562, #3310) Aug 24, 2021

hannes-ucsc pushed a commit that referenced this pull request Aug 24, 2021

[r] Expose 'submission_date' and 'update_date' (#2562, #3310, PR #3026)

384e116

jessebrennan deleted the issues/danielsotirhos/2562-expose-projects-submission-date-and-update-date branch August 24, 2021 23:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose 'submission_date' and 'update_date' (#2562, #3310) #3026

Expose 'submission_date' and 'update_date' (#2562, #3310) #3026

dsotirho-ucsc commented Apr 29, 2021 •

edited by jessebrennan

Loading

codecov bot commented Apr 29, 2021 •

edited

Loading

coveralls commented Apr 29, 2021 •

edited

Loading

hannes-ucsc left a comment

hannes-ucsc left a comment •

edited

Loading

hannes-ucsc May 8, 2021

hannes-ucsc May 8, 2021

hannes-ucsc May 8, 2021

hannes-ucsc May 8, 2021

hannes-ucsc May 8, 2021

hannes-ucsc May 8, 2021

hannes-ucsc May 8, 2021

hannes-ucsc May 8, 2021

hannes-ucsc May 8, 2021

hannes-ucsc left a comment

hannes-ucsc Aug 19, 2021

hannes-ucsc Aug 19, 2021

hannes-ucsc left a comment

jessebrennan commented Aug 24, 2021

jessebrennan commented Aug 26, 2021

		'document_id': str(cell_line.document_id),
		'biomaterial_id': cell_line.biomaterial_id,

	'document_id': str(cell_line.document_id),
	'biomaterial_id': cell_line.biomaterial_id,
	**_biomaterial(cell_line),

		'submission_date': cell_line.submission_date,
		'update_date': cell_line.update_date,

		'submissionDate': p['submission_date'],
		'updateDate': p['update_date'],

Expose 'submission_date' and 'update_date' (#2562, #3310) #3026

Expose 'submission_date' and 'update_date' (#2562, #3310) #3026

Conversation

dsotirho-ucsc commented Apr 29, 2021 • edited by jessebrennan Loading

codecov bot commented Apr 29, 2021 • edited Loading

Codecov Report

coveralls commented Apr 29, 2021 • edited Loading

hannes-ucsc left a comment

Choose a reason for hiding this comment

hannes-ucsc left a comment • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hannes-ucsc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hannes-ucsc left a comment

Choose a reason for hiding this comment

jessebrennan commented Aug 24, 2021

jessebrennan commented Aug 26, 2021

dsotirho-ucsc commented Apr 29, 2021 •

edited by jessebrennan

Loading

codecov bot commented Apr 29, 2021 •

edited

Loading

coveralls commented Apr 29, 2021 •

edited

Loading

hannes-ucsc left a comment •

edited

Loading