Skip to content

Conversation

@cortadocodes
Copy link
Member

@cortadocodes cortadocodes commented May 5, 2021

Contents

New Features

  • Allow Datafile to be used as a context manager for changes to local datafiles
  • Allow Datafile.from_cloud to be used as a context manager for changes to cloud datafiles
  • Allow Datafile to remember where in the cloud it came from
  • Add the following methods to Datafile:
    • get_cloud_metadata
    • update_cloud_metadata
    • clear_from_file_cache
    • _get_cloud_location
    • _store_cloud_location
    • _check_for_attribute_conflict
  • Avoid re-uploading Datafile file or metadata if they haven't changed
  • Raise error if implicit cloud location is missing from Datafile
  • Add GoogleCloudStorageClient.update_metadata method
  • Allow option to not update cloud metadata in Datafile cloud methods

Minor improvements

  • Simplify output of GoogleCloudStorageClient.get_metadata
  • Make Hashable instances re-calculate their hash_value every time unless an immutable_hash_value is explicitly provided (e.g. for cloud datafiles where you don't have the file locally to hash)
  • Add private Identifiable._set_id method

Testing

  • Factor out cloud datafile creation in datafile tests

Quality Checklist

  • New features are fully tested (No matter how much Coverage Karma you have)

@cortadocodes cortadocodes changed the base branch from main to release/0.1.17 May 5, 2021 11:52
@codecov-commenter
Copy link

codecov-commenter commented May 5, 2021

Codecov Report

Merging #157 (cfb4022) into release/0.1.17 (b2aba06) will increase coverage by 0.17%.
The diff coverage is 98.36%.

❗ Current head cfb4022 differs from pull request most recent head 759de13. Consider uploading reports for the commit 759de13 to get more accurate results
Impacted file tree graph

@@                Coverage Diff                 @@
##           release/0.1.17     #157      +/-   ##
==================================================
+ Coverage           94.77%   94.94%   +0.17%     
==================================================
  Files                  55       55              
  Lines                1569     1622      +53     
==================================================
+ Hits                 1487     1540      +53     
  Misses                 82       82              
Impacted Files Coverage Δ
octue/mixins/hashable.py 98.18% <88.88%> (-1.82%) ⬇️
octue/resources/datafile.py 99.02% <98.91%> (+0.92%) ⬆️
octue/cloud/storage/client.py 97.29% <100.00%> (+0.03%) ⬆️
octue/exceptions.py 100.00% <100.00%> (ø)
octue/mixins/identifiable.py 100.00% <100.00%> (ø)

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 b2aba06...759de13. Read the comment docs.

@cortadocodes cortadocodes marked this pull request as ready for review May 5, 2021 21:43
@cortadocodes cortadocodes self-assigned this May 5, 2021
@cortadocodes cortadocodes requested a review from thclark May 5, 2021 21:54
Copy link
Contributor

@thclark thclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems complete in terms of code, but lacking documentation.

I've updated the Architecture slide here to give an image of the four use cases.

Can you add that image (I think you can export as SVG, if not screenshot or png export will do) and and example snippet for each of the four use cases to the documentation please?

@cortadocodes cortadocodes merged commit 6ad67e9 into release/0.1.17 May 7, 2021
@cortadocodes cortadocodes deleted the refactor/consolidate-cloud-datafile-usage branch May 7, 2021 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants