Skip to content
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

♻️ multipart upload chunks are scheduled a few at a time #4552

Merged

Conversation

GitHK
Copy link
Contributor

@GitHK GitHK commented Jul 27, 2023

What do these changes do?

Schedules less chunks to be uploaded wen running multipart uploads.

Related issue/s

How to test

DevOps Checklist

@GitHK GitHK self-assigned this Jul 27, 2023
@GitHK GitHK added t:maintenance Some planned maintenance work changelog:♻️refactor labels Jul 27, 2023
@GitHK GitHK added this to the Sundae milestone Jul 27, 2023
@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Merging #4552 (ff143bf) into master (5f4e812) will increase coverage by 1.4%.
The diff coverage is 84.2%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #4552     +/-   ##
========================================
+ Coverage    84.3%   85.8%   +1.4%     
========================================
  Files         635     811    +176     
  Lines       30346   36257   +5911     
  Branches      776     532    -244     
========================================
+ Hits        25608   31122   +5514     
- Misses       4554    5007    +453     
+ Partials      184     128     -56     
Flag Coverage Δ
integrationtests 63.2% <80.4%> (-5.0%) ⬇️
unittests 83.7% <35.0%> (-0.3%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...src/simcore_sdk/node_ports_common/file_io_utils.py 88.4% <82.6%> (+0.3%) ⬆️
packages/service-library/src/servicelib/utils.py 86.7% <90.9%> (+1.5%) ⬆️

... and 497 files with indirect coverage changes

@GitHK GitHK marked this pull request as ready for review July 27, 2023 11:04
@GitHK GitHK requested a review from pcrespov as a code owner July 27, 2023 11:04
Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

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

A couple of suggestions

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

Cool thanks for this. Also where will you add the overall retry (see DK PR). I think it would also make sense to have it on top.
This one here should allow fail fast, but then the overall retry is also needed

@GitHK
Copy link
Contributor Author

GitHK commented Jul 28, 2023

Cool thanks for this. Also where will you add the overall retry (see DK PR). I think it would also make sense to have it on top.
This one here should allow fail fast, but then the overall retry is also needed

I want this to go to production and see if it makes it better. I want to see if the change has any effect.

Another solution to avoid this problem will be to used rclone, as you have already proposed.

We can wait in adding the retry for a bit. I would still prefer to get away with a "low level" retry and not the retry of the entire file.

Copy link
Contributor

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

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

👍

@GitHK GitHK enabled auto-merge (squash) July 28, 2023 12:23
@codeclimate
Copy link

codeclimate bot commented Aug 2, 2023

Code Climate has analyzed commit ff143bf and detected 0 issues on this pull request.

View more on Code Climate.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 2, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@GitHK GitHK merged commit db2569e into ITISFoundation:master Aug 2, 2023
@GitHK GitHK deleted the pr-osparc-nodeports-high-priority-retry branch August 2, 2023 06:48
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Sep 22, 2023
50 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t:maintenance Some planned maintenance work
Projects
None yet
4 participants