Skip to content

Conversation

@nicoklaus
Copy link
Contributor

@nicoklaus nicoklaus commented Dec 4, 2024

Add new pull mirroring api endpoint released in GitLab 17.6.

https://docs.gitlab.com/17.6/ee/api/project_pull_mirroring.html#configure-pull-mirroring-for-a-project

Changes

Adds mirror_pull_configuration to projects.

Related to #3039

Documentation and testing

Please consider whether this PR needs documentation and tests. This is not required, but highly appreciated:

Additional Notes

Since the implementation of this API is slightly different from the standard that GitLab uses, there are small drawbacks to disabling pull mirrors.

There is no separate DELETE method, we use the PUT method in both cases to enable and disable pull mirroring. Since a URL is required for activation, this is now also required for deactivation, even though it is not actually needed for this.

I also noticed that the settings for mirror_regex are not deleted when deactivated, unlike the other settings. Therefore there is now an extra if condition for this. I'm not sure if this should be filed as a question / bug to GitLab ?

@codecov
Copy link

codecov bot commented Dec 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.80%. Comparing base (30f470b) to head (7e59c21).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3056      +/-   ##
==========================================
+ Coverage   96.79%   96.80%   +0.01%     
==========================================
  Files          97       97              
  Lines        6325     6346      +21     
==========================================
+ Hits         6122     6143      +21     
  Misses        203      203              
Flag Coverage Δ
api_func_v4 83.07% <85.71%> (+0.05%) ⬆️
cli_func_v4 83.04% <66.66%> (-0.06%) ⬇️
unit 89.34% <100.00%> (+0.03%) ⬆️

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

Files with missing lines Coverage Δ
gitlab/v4/objects/projects.py 98.93% <100.00%> (+0.06%) ⬆️

@nejch
Copy link
Member

nejch commented Dec 5, 2024

Thanks a lot @nicoklaus, looks great in general!

I'm just thinking whether we should consolidate the GET, PUT and POST methods for /pull/mirror in a single manager+object so that we don't need the custom handling here, and that way people can just treat things like .enabled as another attribute to call .save() on.

Just have to think about a clean way to do this, maybe POST would make more sense as a .start() method. I'll come back to you with some ideas ASAP 🙇

@nicoklaus nicoklaus closed this Jan 23, 2025
@nicoklaus nicoklaus reopened this Jan 23, 2025
@nicoklaus
Copy link
Contributor Author

Hi @nejch, I had a little time and reworked the implementation based on your input. 🔨 would be pleased about another review 😄

Copy link
Member

@nejch nejch left a comment

Choose a reason for hiding this comment

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

Hi @nejch, I had a little time and reworked the implementation based on your input. 🔨 would be pleased about another review 😄

Thanks a lot @nicoklaus, this is exactly what I had in mind. Looks great! Just a comment on the impact of removing the old code, let me know if you need some help 🙇

@nicoklaus nicoklaus requested a review from nejch January 28, 2025 09:17
@nejch nejch merged commit 7f6fd5c into python-gitlab:main Jan 28, 2025
18 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