Skip to content

Fix copy_repo sending incorrect repo pairs when providing multiple pairs#333

Merged
rajulkumar merged 1 commit intorelease-engineering:masterfrom
rajulkumar:fix_copy_repo_pairs
Jan 9, 2025
Merged

Fix copy_repo sending incorrect repo pairs when providing multiple pairs#333
rajulkumar merged 1 commit intorelease-engineering:masterfrom
rajulkumar:fix_copy_repo_pairs

Conversation

@rajulkumar
Copy link
Collaborator

Copy_repo waits for the complete copy of non-rpm content before copying rpm content. However, there were some instances where previous repo pair was sent for non-rpm content copy when multiple repo pairs were provided.

Lambda functions seems to be the reason for the error as they resolve the values during execution rather than creation, hence might be picking incorrect values. This patch replaces it with a modified partial function method that resolves the values during creation instead of execution.

@rajulkumar rajulkumar requested a review from rohanpm as a code owner January 7, 2025 07:01
@rajulkumar rajulkumar force-pushed the fix_copy_repo_pairs branch from 23f2ac3 to c08eb6d Compare January 7, 2025 07:03
@codecov
Copy link

codecov bot commented Jan 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (4212bf1) to head (6fedeac).
Report is 9 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #333   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           54        54           
  Lines         3011      3014    +3     
=========================================
+ Hits          3011      3014    +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rajulkumar rajulkumar requested a review from rbikar January 7, 2025 07:12
rohanpm
rohanpm previously approved these changes Jan 7, 2025
"""The repo to which content was copied."""


class partpartial(partial):
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: I am not really fond of the name, because to me it implies a weaker kind of partial ("part partial"), while this is actually stronger than a usual partial object. It freezes all the arguments, while I think partial's name comes from the fact that it's typically used to partially freeze/bind a function's arguments.

Not a big deal obviously, given it's not public API.

Some other ideas for names: freeze_arguments, bind_arguments, full_partial.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the insight.
"Part partial" came from the fact that it's not providing all the features of a "partial", hence the name. However, suggested names sounds more meaningful.

Copy_repo waits for the complete copy of non-rpm content before
copying rpm content. However, there were some instances where
previous repo pair was sent for non-rpm content copy when multiple
repo pairs were provided.
Lambda functions seems to be the reason for the error as they
resolve the values during execution rather than creation, hence
might be picking incorrect values. This patch replaces it with a
modified partial function method that resolves the values during
creation instead of execution.
@rajulkumar rajulkumar merged commit dc12f74 into release-engineering:master Jan 9, 2025
9 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.

2 participants