Skip to content

Fix saving large pipelines #1335

Merged
merged 3 commits into from
Jul 31, 2023
Merged

Fix saving large pipelines #1335

merged 3 commits into from
Jul 31, 2023

Conversation

Mr-Geekman
Copy link
Contributor

@Mr-Geekman Mr-Geekman commented Jul 27, 2023

Before submitting (must do checklist)

  • Did you read the contribution guide?
  • Did you update the docs? We use Numpy format for all the methods and classes.
  • Did you write any new necessary tests?
  • Did you update the CHANGELOG?

Proposed Changes

Add option force_zip64 during writing to archive.

Closing issues

Closes #1318.

@Mr-Geekman Mr-Geekman self-assigned this Jul 27, 2023
@github-actions
Copy link

github-actions bot commented Jul 27, 2023

@github-actions github-actions bot temporarily deployed to pull request July 27, 2023 12:33 Inactive
@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2023

Codecov Report

Merging #1335 (2183f07) into master (e4c121c) will decrease coverage by 0.15%.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master    #1335      +/-   ##
==========================================
- Coverage   89.09%   88.95%   -0.15%     
==========================================
  Files         204      193      -11     
  Lines       12636    12321     -315     
==========================================
- Hits        11258    10960     -298     
+ Misses       1378     1361      -17     
Files Changed Coverage Δ
etna/core/mixins.py 95.90% <100.00%> (ø)
etna/models/mixins.py 96.26% <100.00%> (ø)

... and 11 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -230,7 +230,7 @@ def _save_metadata(self, archive: zipfile.ZipFile):
output_file.write(metadata_bytes)

def _save_state(self, archive: zipfile.ZipFile):
with archive.open("object.pkl", "w") as output_file:
with archive.open("object.pkl", "w", force_zip64=True) as output_file:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it affects loading somehow? Do we have test for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't affect loading, we have tests on saving/loading (not large files). Large files I tested manually.
As I understand the problem that with archive.open is that is should know about the size of the file to create a correct metainfo. We provide it with force_zip64 to make sure it creates metainfo for large file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@github-actions github-actions bot temporarily deployed to pull request July 31, 2023 06:41 Inactive
@Mr-Geekman Mr-Geekman merged commit cceb500 into master Jul 31, 2023
12 checks passed
@Mr-Geekman Mr-Geekman deleted the issue-1318 branch July 31, 2023 07:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Can't save large pipeline
3 participants