feat: ability to transfer project between groups or namespaces #582

amimas · 2023-08-01T17:39:17Z

This PR implements the ability to transfer a project in Gitlab. Project can be transferred from one namespace to another including subgroups. A new config key (transfer_from) has been introduced. See this comment for config syntax proposal/reason.

This feature uses python-gitlab for making API calls to Gitlab instead of expanding homegrown API. A new class is added so that it can be used for existing/future features.

closes #213

codecov · 2023-08-01T17:42:22Z

Codecov Report

Merging #582 (508cc73) into main (d3e3187) will increase coverage by 0.33%.
The diff coverage is 93.87%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #582      +/-   ##
==========================================
+ Coverage   83.16%   83.50%   +0.33%     
==========================================
  Files          70       70              
  Lines        2703     2746      +43     
==========================================
+ Hits         2248     2293      +45     
+ Misses        455      453       -2

Files	Coverage Δ
gitlabform/processors/project/project_processor.py	`100.00% <100.00%> (ø)`
gitlabform/gitlab/__init__.py	`98.11% <90.90%> (-1.89%)`	⬇️
gitlabform/lists/projects.py	`94.87% <81.81%> (+0.75%)`	⬆️

... and 1 file with indirect coverage changes

gitlabform/gitlab/__init__.py

amimas · 2023-08-01T18:00:11Z

Surprisingly the smoke test is failing in CI even though the acceptance tests are passing. The smoke test is passing on my local machine too; running on python 3.10.

amimas · 2023-08-01T19:35:25Z

Surprisingly the smoke test is failing in CI even though the acceptance tests are passing. The smoke test is passing on my local machine too; running on python 3.10.

Needed to set python-gitlab as runtime dependency. duh... 🤦‍♂️ . It's resolved now.

gdubicki

Looks great, thank you for the contribution, and sorry for the late review, @amimas!

gitlabform/gitlab/__init__.py

amimas · 2023-08-25T12:20:45Z

gitlabform/processors/project/project_processor.py

+                except GitlabTransferProjectError:
+                    fatal(
+                        "Encountered error transferring project. Please check if project transfer requirements were met. Docs: https://docs.gitlab.com/ee/user/project/settings/index.html#transfer-a-project-to-another-namespace"
+                    )
+                    raise


Thanks for the review @gdubicki . I was taking a final glance before merging and a thought occurred.

Should this be a fatal error? Will gitlabform continue processing other projects or will it exit here?

Following the example here:

gitlabform/gitlabform/lists/projects.py

Lines 112 to 116 in fd903cb

except NotFoundException:

fatal(

f"Configuration contains project {project} but it cannot be found in GitLab!",

exit_code=EXIT_INVALID_INPUT,

)

I guess it does make sense to use fatal but I shouldn't re-raise the exception?

Right, you should not re-raise it.

I forgot a little how it worked but I refreshed my memory. We have 3 kinds of errors in gitlabform:

Regular exceptions - these will break gitlabform run if --terminate flag is set in cli, or will just break processing given group or project if not - see this code,

Unconditionally fatal errors - if fatal() is called without an if, for example here,

Conditionally fatal errors - if --strict flag is set in cli, if will break a gitlabform run, otherwise it will not. It's restricted to some special cases of given entity, usually a branch, not existing. See this code as an example.

It may seem more chaotic than it actually is.

There is value in having a separate flag for NOT allowing some entities to not exist (--strict) and allowing that by default, because in one of the main use cases of this app I used to manage tens of repositories in a single group where some had only master branch, some had develop and master and I could have a single config that did some things with both branch but not fail if develop did not exist.

But probably we could make it more consistent by checking all the error cases and maybe expanding the cases that --strict covers. 🤔

Thanks @gdubicki for the details. I'll look into it and update my exception handling.

Hi @gdubicki - I think in this case it makes sense for this to be handled based on --tarminate flag. What do you think? I've never had it enabled in my actual usage though. And to be honest, I never quite understood the --strict flag (i.e. why another flag in addition to --terminate)?. Anyways, I think that's a discussion for separate issue or possibly for next major version design.

Unfortunately I couldn't figure out how to check for the --terminate flag in the above exception handling. From the ProjectProcessor, I'm not sure how to access it. So, I've just set it as a "warning" message for now. Could you please help when you get a chance? Thank you!

After consideration I think you should not catch the GitlabTransferProjectError exception at all. If you won't do that, then this code with catch it and either break the run (if --terminate flag is set) or just break processing given project.

Thanks for looking into it @gdubicki . In that case, wouldn't the original setup have worked because I had re-raised the exception after logging a message? Should I revert to that code or do you think simply remove the exception catching/handling?

I think that to be consistent with the currenr approach you should remove the whole try block.

A better solution would be to do what you do now to show the custom error message and then re-raise the exception but wrapped in a class that would prevent from reprinting the exception again (of course that would have to be implemented).

Wdyt?

I agree with you. I'll push an update and remove the exception catching so that we're consistent.

We can open a separate issue with a proposal for improving the logging of exception handling in similar cases.

Opened a separate issue to keep track of this: #611

I've removed the try and except that was catching the exception earlier. Well... I've commented them out as "Todo". I also realized that the acceptance for it wasn't capturing the custom message. It would pass regardless of what I wrote. Instead of removing the test case, I marked it to be skipped for now. It should probably be enabled and fixed later when exception catching/handling is done with custom messages as per above issue/idea.

Please review one more time.

amimas · 2023-09-30T00:29:15Z

I wonder how dry-run/noop parameter will process this feature. I'm not yet fully familiar with the franework or process in place for dryrun in this project.

gdubicki

Please also see the 2 minor comments about the code.

gdubicki · 2023-10-07T12:33:24Z

gitlabform/processors/project/project_processor.py

+                        "Updating the source project path from '"
+                        + project_to_be_transferred.path
+                        + "' to '"
+                        + destination_project_path


Nit: single quotation mark is not closed here.

You might consider replacing those multiple lines with f-strings, which might make the code more readable and such mistakes more rare.

Thank you. I'll look into it.

I've pushed an update to address it. Please have a look.

gdubicki · 2023-10-07T12:47:54Z

gitlabform/processors/project/project_processor.py

+                except GitlabTransferProjectError:
+                    fatal(
+                        "Encountered error transferring project. Please check if project transfer requirements were met. Docs: https://docs.gitlab.com/ee/user/project/settings/index.html#transfer-a-project-to-another-namespace"
+                    )
+                    raise


After consideration I think you should not catch the GitlabTransferProjectError exception at all. If you won't do that, then this code with catch it and either break the run (if --terminate flag is set) or just break processing given project.

…reference

amimas requested a review from gdubicki August 1, 2023 17:39

amimas temporarily deployed to Integrate Pull Request August 1, 2023 17:39 — with GitHub Actions Inactive

amimas temporarily deployed to Integrate Pull Request August 1, 2023 17:52 — with GitHub Actions Inactive

amimas commented Aug 1, 2023

View reviewed changes

gitlabform/gitlab/__init__.py Show resolved Hide resolved

amimas temporarily deployed to Integrate Pull Request August 1, 2023 18:06 — with GitHub Actions Inactive

amimas temporarily deployed to Integrate Pull Request August 1, 2023 19:34 — with GitHub Actions Inactive

amimas temporarily deployed to Integrate Pull Request August 1, 2023 21:05 — with GitHub Actions Inactive

amimas temporarily deployed to Integrate Pull Request August 2, 2023 04:03 — with GitHub Actions Inactive

amimas temporarily deployed to Integrate Pull Request August 2, 2023 07:02 — with GitHub Actions Inactive

gdubicki approved these changes Aug 25, 2023

View reviewed changes

gitlabform/gitlab/__init__.py Show resolved Hide resolved

amimas force-pushed the issue-213 branch from 40cfff2 to 9c1c649 Compare August 25, 2023 12:02

amimas temporarily deployed to Integrate Pull Request August 25, 2023 12:02 — with GitHub Actions Inactive

amimas commented Aug 25, 2023

View reviewed changes

amimas temporarily deployed to Integrate Pull Request August 26, 2023 21:18 — with GitHub Actions Inactive

amimas force-pushed the issue-213 branch from 6b821e7 to d59cb23 Compare September 7, 2023 02:37

amimas temporarily deployed to Integrate Pull Request September 7, 2023 02:37 — with GitHub Actions Inactive

amimas requested a review from gdubicki September 7, 2023 02:41

amimas force-pushed the issue-213 branch from d59cb23 to 7c3f5a3 Compare September 9, 2023 14:06

amimas temporarily deployed to Integrate Pull Request September 9, 2023 14:06 — with GitHub Actions Inactive

amimas force-pushed the issue-213 branch from 7c3f5a3 to a8c8d77 Compare September 16, 2023 14:47

amimas temporarily deployed to Integrate Pull Request September 16, 2023 14:47 — with GitHub Actions Inactive

amimas force-pushed the issue-213 branch from a8c8d77 to 29e009d Compare September 26, 2023 02:51

amimas temporarily deployed to Integrate Pull Request September 26, 2023 02:51 — with GitHub Actions Inactive

amimas force-pushed the issue-213 branch from 29e009d to 866d9b8 Compare October 3, 2023 01:11

amimas temporarily deployed to Integrate Pull Request October 3, 2023 01:11 — with GitHub Actions Inactive

gdubicki reviewed Oct 7, 2023

View reviewed changes

amimas had a problem deploying to Integrate Pull Request October 8, 2023 23:38 — with GitHub Actions Failure

amimas mentioned this pull request Oct 10, 2023

Improve logging by catching exceptions and printing helpful custom messages #611

Open

amimas added 6 commits October 10, 2023 20:19

feat: ability to transfer project between groups or namespaces

894ae14

chore(deps): set python-gitlab as runtime dependency

38c943e

fix: set project transfer error as warning instead of raising exception

87406d8

chore(deps): re-org dependencies alphabatically and remove duplicate …

9319a1a

…reference

chore: use f-string for log messages

2032836

chore: remove project transfer exception handling to as per PR review

7964de0

amimas force-pushed the issue-213 branch from e5c81b5 to 7964de0 Compare October 11, 2023 00:24

amimas temporarily deployed to Integrate Pull Request October 11, 2023 00:24 — with GitHub Actions Inactive

amimas requested a review from gdubicki October 11, 2023 00:28

style: update code style via black

508cc73

amimas temporarily deployed to Integrate Pull Request October 11, 2023 00:30 — with GitHub Actions Inactive

gdubicki approved these changes Oct 12, 2023

View reviewed changes

amimas temporarily deployed to Integrate Pull Request October 12, 2023 18:37 — with GitHub Actions Inactive

amimas merged commit d56ab32 into gitlabform:main Oct 12, 2023
20 checks passed

amimas mentioned this pull request Oct 12, 2023

More Contributors / Developers / Maintainers needed! #343

Open

amimas mentioned this pull request Oct 26, 2023

refactor: migrate project:archive feature to use python-gitlab library #622

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: ability to transfer project between groups or namespaces #582

feat: ability to transfer project between groups or namespaces #582

amimas commented Aug 1, 2023

codecov bot commented Aug 1, 2023 •

edited

Loading

amimas commented Aug 1, 2023

amimas commented Aug 1, 2023

gdubicki left a comment

amimas Aug 25, 2023

amimas Aug 25, 2023

gdubicki Aug 26, 2023

amimas Aug 26, 2023

amimas Aug 26, 2023

gdubicki Oct 7, 2023

amimas Oct 8, 2023

gdubicki Oct 9, 2023

amimas Oct 9, 2023

amimas Oct 11, 2023

amimas commented Sep 30, 2023

gdubicki left a comment

gdubicki Oct 7, 2023

gdubicki Oct 7, 2023

amimas Oct 8, 2023

amimas Oct 11, 2023

gdubicki Oct 7, 2023

	except NotFoundException:
	fatal(
	f"Configuration contains project {project} but it cannot be found in GitLab!",
	exit_code=EXIT_INVALID_INPUT,
	)

feat: ability to transfer project between groups or namespaces #582

feat: ability to transfer project between groups or namespaces #582

Conversation

amimas commented Aug 1, 2023

codecov bot commented Aug 1, 2023 • edited Loading

Codecov Report

amimas commented Aug 1, 2023

amimas commented Aug 1, 2023

gdubicki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amimas commented Sep 30, 2023

gdubicki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Aug 1, 2023 •

edited

Loading