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

fix: dry run failure when project transfer is configured along with other additional configs #726

Merged
merged 7 commits into from Apr 12, 2024

Conversation

long-wan-ep
Copy link
Contributor

Fixes #725.

Project transfer config can fail when run with the --noop flag, if a processor's _print_diff method makes a GitLab API call for the not yet existing project. This PR adds logic to check for transfer_from and makes the API call using the correct project path depending on the situation.

@long-wan-ep long-wan-ep had a problem deploying to Integrate Pull Request April 9, 2024 21:17 — with GitHub Actions Failure
Copy link

codecov bot commented Apr 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.06%. Comparing base (ec6ba78) to head (dfb2d17).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #726      +/-   ##
==========================================
- Coverage   84.35%   84.06%   -0.29%     
==========================================
  Files          69       69              
  Lines        2774     2780       +6     
==========================================
- Hits         2340     2337       -3     
- Misses        434      443       +9     
Files Coverage Δ
gitlabform/__init__.py 62.01% <100.00%> (-3.11%) ⬇️
gitlabform/processors/abstract_processor.py 84.00% <100.00%> (-2.18%) ⬇️

... and 5 files with indirect coverage changes

@long-wan-ep long-wan-ep had a problem deploying to Integrate Pull Request April 9, 2024 21:31 — with GitHub Actions Failure
Copy link
Collaborator

@amimas amimas left a comment

Choose a reason for hiding this comment

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

It feels like the logic (setting current project to the value set by transfer_source) will need to be repeated.. wherever the diff is supported. Otherwise this bug will continue to show up. For example: let's say diff is supported for branches or members etc. If the config contains project to be transferred first, when we do dry-run/noop, wouldn't we have the same issue again?

gitlabform/processors/single_entity_processor.py Outdated Show resolved Hide resolved
gitlabform/processors/abstract_processor.py Outdated Show resolved Hide resolved
@long-wan-ep long-wan-ep temporarily deployed to Integrate Pull Request April 11, 2024 21:25 — with GitHub Actions Inactive
Copy link
Collaborator

@amimas amimas left a comment

Choose a reason for hiding this comment

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

Latest change looks good. Just one small change needed I think.

gitlabform/processors/abstract_processor.py Show resolved Hide resolved
@long-wan-ep long-wan-ep temporarily deployed to Integrate Pull Request April 11, 2024 21:54 — with GitHub Actions Inactive
@long-wan-ep long-wan-ep temporarily deployed to Integrate Pull Request April 12, 2024 16:23 — with GitHub Actions Inactive
@long-wan-ep long-wan-ep temporarily deployed to Integrate Pull Request April 12, 2024 18:05 — with GitHub Actions Inactive
Copy link
Collaborator

@amimas amimas left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you for the fix.

@amimas amimas changed the title fix: project transfer with noop fix: dry run failure when project transfer is configured along with other additional configs Apr 12, 2024
@amimas amimas merged commit 71070bf into gitlabform:main Apr 12, 2024
23 checks passed
@amimas
Copy link
Collaborator

amimas commented Apr 15, 2024

Released in v3.9.4

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.

Project transfer failing with --noop
2 participants