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

Properly use concurrent_transfers from MedusaConfig object #1244

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

rzvoncek
Copy link
Contributor

What this PR does:
This PR changes the default value to 0 (instead of 1). This way, the merging storage properties, the correct value wins.
This PR also makes sure that if we do end up with a 0, the medusa.ini ends up with a 1 because Medusa can't handle 0 concurrent transfers.

Which issue(s) this PR fixes:
Fixes #1239

Checklist

  • Changes manually tested
  • Automated Tests added/updated
  • Documentation added/updated
  • CHANGELOG.md updated (not required for documentation PRs)
  • CLA Signed: DataStax CLA

@rzvoncek rzvoncek requested a review from a team as a code owner March 13, 2024 15:40
@rzvoncek rzvoncek force-pushed the radovan/concurrent-transfers branch from 12871d9 to f3633c6 Compare March 15, 2024 12:18
Copy link

codecov bot commented Mar 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 57.30%. Comparing base (94bc1d8) to head (57daa2f).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1244      +/-   ##
==========================================
- Coverage   57.34%   57.30%   -0.04%     
==========================================
  Files         103      103              
  Lines       10790    10794       +4     
==========================================
- Hits         6188     6186       -2     
- Misses       4064     4069       +5     
- Partials      538      539       +1     
Files Coverage Δ
pkg/medusa/reconcile.go 60.94% <100.00%> (+0.28%) ⬆️

... and 3 files with indirect coverage changes

@rzvoncek rzvoncek force-pushed the radovan/concurrent-transfers branch from f3633c6 to 57daa2f Compare March 15, 2024 13:49
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
64.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

Copy link
Contributor

@adejanovski adejanovski left a comment

Choose a reason for hiding this comment

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

Perfect, thanks!

@rzvoncek rzvoncek merged commit bd7ca25 into main Mar 15, 2024
60 of 62 checks passed
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.

Concurrent transfers from a MedusaConfiguration is always overridden by 1
2 participants