Consider migrating to `python-gitlab`

[gitlabform]

Consider migrating to `python-gitlab`

[max-wittig]

Maintainer of python-gitlab here. That would totally make sense. Contributions welcome. Let me know, if you need help!

[glensc]

There's a (small) point of porting, to have a similar configurability: i.e read the python-gitlab.cfg in a known format and locations.

[gdubicki]

Thank for the suggestions, @glensc .

And thanks for expanding it, @nejch ! Idea of using lower-level python-gitlab methods does sound useful - apart from reusing the semi-standard .cfg file we would also probably not run into issues such as #90 and in the future possibly move to upper-level code.

I would be happy if you could do it though as we already started planning v2.0 that should be ready around Jan/Feb 2021 as I don't really want to add more to the scope of that version.

So this can wait for your readiness. :)

[gdubicki]

Hey @nejch!

Just a friendly reminder that if you now have the time to try to code what you suggested, then please go ahead. :)

[lfvjimisola]

GitLab are making quite a change with merging Projects and Groups into Namespaces, Issues/Epics etc into Work Items.

See: my post here

So, it probably makes even more sense to use python-gitlab with all changes coming up. gitlabform will be more a YAML-frontend to python-gitlab then.

[nejch]

This is still on my mind as well, just wanted to add. At python-gitlab we'd also benefit from it as we could implement things together then. I should start this again then :)

[amimas]

Hi @gdubicki , @nejch - I'm trying to help with migrating to python-gitlab library. I created this project (https://github.com/orgs/gitlabform/projects/1/views/1) so that we can iterate on it instead of a big change. This way, others can help where possible.

Currently gitlabform has the concept of SingleEntityProcessor and MultipleEntitiesProcessor. I think the logics that are using those 2 classes probably have to be migrated at the same time. Not sure there's a way to make the change in smaller scope. I'm not a python expert.

I'm also not sure if those classes are still usable if we go with python-gitlab library.

For example, the project_settings configuration uses the SingleEntityProcessor and sets the name of method to be called for updating project settings via GitLab API.

gitlabform/gitlabform/processors/project/project_settings_processor.py

Lines 5 to 12 in c96202a

	class ProjectSettingsProcessor(SingleEntityProcessor):
	def __init__(self, gitlab: GitLab):
	super().__init__(
	"project_settings",
	gitlab,
	get_method_name="get_project_settings",
	edit_method_name="put_project_settings",
	)

The SingleEntityProcessor then calls that method when update is needed:

gitlabform/gitlabform/processors/single_entity_processor.py

Lines 40 to 43 in c96202a

	if self._needs_update(entity_in_gitlab, entity_config):
	verbose(f"Editing {self.configuration_name} in {project_or_group}")
	self.edit_method(project_or_group, entity_config)
	debug(f"{self.configuration_name} AFTER: ^^^")

This then calls the following method with the config options:

gitlabform/gitlabform/gitlab/projects.py

Lines 129 to 142 in c96202a

	def put_project_settings(self, project_and_group_name, project_settings):
	# project_settings has to be like this:
	# {
	# 'setting1': value1,
	# 'setting2': value2,
	# }
	# ..as documented at: https://docs.gitlab.com/ce/api/projects.html#edit-project
	return self._make_requests_to_api(
	"projects/%s",
	project_and_group_name,
	"PUT",
	data=None,
	json=project_settings,
	)

I'm not sure if this approach can be used with python-gitlab library or how. For updating project settings using python-gitlab library, I think it's done by something like this:

# get project first and then change properties
project.visibility = "internal"
project.description = "some description"
...
# save the setting changes
project.save()

I haven't played with the python-library enough yet. So, not sure if there's another option.

Anyways, I'm just trying to wrap my head around it. As I'm still a python beginner :), I think the above will need some help from other experts. Or, if you know exactly how we can do the migration of those configurations, maybe help by opening issue with detailed steps.

[weakcamel]

For what it's worth, I've used python-gitlab for a few minor tasks and it usually does the job well. Maintainers do a great job.

Once or twice it did go out of sync with Gitlab's API - but that's only to be expected with a relatively unstable interface that Gitlab provides. That said, there are certain functionalities (from EE tiers) which they may not be interested in implementing (such as e.g. python-gitlab/python-gitlab#887) so keeping a fallback option might be a good thing.

[gdubicki]

Hey @max-wittig , thanks a lot for pinging us here!

I think that that we could migrate 90% of GitLabForm to python-gitlab - the exceptions are the places where we by design pass the parameters from YAML as they are to the GitLab API endpoints (see https://github.com/egnyte/gitlabform/blob/master/FEATURES_DESIGN.md#raw-parameters-passing ).

Also we could do the migration gradually, firstly when adding new features or editing exisiting ones and then at some point complete the migration.

Perhaps you could help with using python-gitlab for implementing #106 ? Please see my last comments there for the issues I have found in the GitLab APIs. Do you have the problems of getting a list of users with personal projects and when you have a user a list of their projects solved in python-gitlab?

[gdubicki]

I created a quick and dirty try to use python-gitlab instead of own library here: https://github.com/egnyte/gitlabform/pull/121/files

Most of the changed lines is the boilerplate. The core change is the process_secret_variables function:

• the old one,
• the new one,

Can you take a look at it please, @max-wittig ? Do you think this is this the smartest way I can use python-gitlab for such use case?

[gdubicki]

After trying this out in #121 I would say that it didn't work out.

We are using the GitLab API on a pretty low level and the wrappers that the python-gitlab provides are not helping us.

There is also a really low cost of adding and maintaining support for the GitLab API ourselves (see how little code do we have in https://github.com/egnyte/gitlabform/tree/master/gitlabform/gitlab ).

Therefore I am closing this as "won't do", as of now.

[nejch]

Just my two cents: if you do still consider this in the future, e.g. to reuse .cfg files as stated above, python-gitlab does provide lower-level methods that all wrapper methods use, but you can also use for any non-implemented features. These are all available in instances of Gitlab so with gl = gitlab.Gitlab() you get:

gl.http_get()
gl.http_list() (a helper GET for lists)
gl.http_post()
gl.http_put()
gl.http_delete()

These in turn call a lower-level method, which you can also use as gl.http_request() and seems to be most similar to your _make_request_to_api() from a quick glance in your repo. Quite a few people (including myself in the past) use these methods for new features before they land in python-gitlab.

Not sure if #121 shows the benefits of reusing this, maybe the first step would be to make your core gitlab functions just wrappers around gl.<resource>.<verb> and go from there by slowly removing boilerplate. I could maybe give it a shot at some point but it would be probably have to be in 2021 as I'm currently buried in some non-python stuff :)

[nejch]

Hey @nejch!

Just a friendly reminder that if you now have the time to try to code what you suggested, then please go ahead. :)

Thanks! I've started something on the weekend. python-gitlab explicitly raises exceptions when status codes are not 2xx unlike your expected_codes approach that is sometimes fine with 404 etc. So it will be a bit of a bigger PR than I expected initially but will post it here when I have a draft :)

[gdubicki]

Cool, thanks @nejch ! Looking forward to your PR. 😊

Just please note that accepting various codes in our current code, such as 404 for example, is a consequence of a decision to not check if a resource exists before trying to delete it. F.e. it's like this in delete_secret_variable() because the code for processing secret variables deletion looks like this.

I assumed that this is a better approach for 2 reasons:

• it seems to be more practical - even in our tests cleanup there were cases where we tried to delete resources when they were already deleted and instead of adding the code to check if they exist in multple places we just added 404 as an accepted code for deleting methods,
• it seems to be more idempotent to me - of course this is arguable, but I think that it makes the method calling GitLab API more similar to a code in Ansible or Puppet, where declare that you want to something to not exist and you don't really care if it didn't exist already or if it did exist and was deleted.

I am open to agreeing to a different approach if there is a good reason to do this, but it would be good to have this discussion before you put a lot of effort into the PR that we may fundamentally disagree on. 😅

Feel invited to our slack workspace where we can discuss this faster. :) Here is an invite link for you: https://join.slack.com/t/gitlabform/shared_invite/zt-ljgzvxk0-TubjJFA5lLpRkyivQWI_Qg

(Of course everyone else who wants to chip in here or into other GitLabForm development or usage related talks is welcome too. 😊)

[nejch]

Oh no I I completely agree :) I just wanted to give a quick update to my previous post, that it wasn't a 1-to-1 replacement for _make_request_to_api but it's basically just instead of if code in expected_codes it might need to be try.. except ExpectedException1,2,3... and then handle the lack of a standard requests response structure due to the exception in python-gitlab (or something like that 😁) I also usually put my stuff into try/except blocks when using python-gitlab for the exact same reason.

This is also how some of the Ansible gitlab modules (which also use python-gitlab) do it, try creating first, and update on exceptions instead (and I guess similar for deletion). for example https://github.com/ansible-collections/community.general/blob/f3e640d5a0b722e00ee835155f45a56d1e5e2c64/plugins/modules/source_control/gitlab/gitlab_runner.py#L186-L258

Maybe a context manager or decorator to give expected exceptions when resources already/no longer exist would make sense to keep it clean and similar to the current approach.

[gdubicki]

Hey again @nejch !

In my recent work on generalizing the code (#250) I got to a point where I started to:

• define the attributes that are used for matching objects in GitLabForm's configuration and the ones returned from GitLab API - I called them "defining" and use a name "key" instead of "attribute",
• define the expression that says what attributes are needed to create or update an object - see the "required_to_create_or_update" variables.

I wrote that for Project Badges, Group Badges and Group LDAP Links, see:

• https://github.com/egnyte/gitlabform/pull/250/files#diff-2a62143b0e95ac85ef830922ee2d8bf5588301179cf42c58ae921d4fc6c2c609R14-R17
• https://github.com/egnyte/gitlabform/pull/250/files#diff-450e1f5f29af277b585c46e57326bba3b23e27332d8b527de9d7691e688be333R14-R17,
• https://github.com/egnyte/gitlabform/pull/250/files#diff-90a936fad43bb346b1513111eed0eba2a87c6fadbbc99472c9f5f487980ea62cR14-R17

After sleeping over it I realized that this is the point where I would have to rewrite a lot of my code to be really OOP to avoid an ugly mess of OOP and de facto procedural code, that I have now. And after a bit of thinking, I came to the idea that maybe this is the moment to re-try using python-gitlab's RESTObject and RESTManager, instead of re-writing it?

For now I am looking over python-gitlab code again and trying to hack some POC.

[gdubicki]

Don't worry that PR #250 is already merged. As my acceptance tests were passing and I have been preparing this release for some time now, I wanted to finally get it released for the users. But I am still very open to refactor this code in a new PR. :)

[nejch]

Started working on #442 finally. It only touches code relevant to tests. Still needs some cleanup and premium tests but it's a massive change so thought I'd start somewhere:)