Add GitLab fetcher #649

suhanoves · 2024-03-17T21:18:58Z

Proposed changes

Hello Augusto,
Thank you for developing such a fantastic library!

I've been working on incorporating the ability to store style files on a self-hosted GitLab instance, which I noticed was missing from the library, so I took the initiative to add this feature.

I've made an effort to thoroughly document the nuances of interfacing with GitLab's API in the method docstrings. Unlike GitHub, GitLab's API relies on project IDs, while the GitLab website uses project paths. This distinction necessitated implementing functionality to support both behaviors.

I faced some challenges setting up the project, as I was unfamiliar with tox.

Currently, I've managed to write tests only for the URL generation, not for the Fetcher itself. I'm unsure which existing tests cover similar functionality.

If you could provide some guidance, I would be eager to fully complete the merge request. I'm relatively new to contributing to open source and would appreciate your help.

Checklist

Read the contribution guidelines
Run make locally before pushing commits
Add tests for the relevant parts:
- API
- CLI
- flake8 plugin (normal mode)
- flake8 plugin (offline mode)
Write documentation when there's a new API or functionality

andreoliwa · 2024-03-23T21:09:29Z

Hey, thanks for this PR! 🙏🏻
I'm a bit busy at the moment but I will take a look as soon as possible.

for more information, see https://pre-commit.ci

andreoliwa · 2024-04-12T23:31:09Z

docs/configuration.rst

Now at least the build is fixed.
For reference, here is the last build of lines with missing coverage.

We can try to mimic how GitHub was tested.

andreoliwa · 2024-04-12T23:32:31Z

docs/configuration.rst

@@ -42,6 +42,9 @@ Remote style

 Use the URL of the remote file.

+Github


It's a capital "H". 😅

Suggested change

Github

GitHub

Please fix it everywhere if there are other places.

andreoliwa · 2024-04-12T23:33:26Z

docs/configuration.rst

@@ -111,6 +114,85 @@ Or you can use an environment variable to avoid keeping secrets in plain text.
    A literal token cannot start with a ``$``.
    All tokens must not contain any ``@`` or ``:`` characters.

+Gitlab


Same thing here, capital "L". 😄

Suggested change

Gitlab

GitLab

andreoliwa · 2024-04-12T23:37:48Z

src/nitpick/style.py

@@ -1,3 +1,4 @@
+# pylint: disable=too-many-lines # TODO: refactor: break this into separate modules in a follow-up PR


I added this and will fix it later on another PR, so this one stays small.

andreoliwa · 2024-04-12T23:40:06Z

docs/configuration.rst

+    A literal token cannot start with a ``$``.
+    All tokens must not contain any ``@`` or ``:`` characters.
+
+


I read this far.
Good docs, thanks a lot. 👍🏻 👏🏻

I will try it out on some GitLab repo from gitlab.com (I don't have a self-hosted or paid versions).

andreoliwa · 2024-04-12T23:42:14Z

src/nitpick/style.py

+def raise_gitlab_incorrect_url_error(url: furl) -> NoReturn:
+    """Raise an error if the URL is not a valid GitLab URL."""
+    message = f"Invalid GitLab URL: {url}"
+    raise ValueError(message)


This could be a private method on the GitLabURL below since it's only used there.

andreoliwa · 2024-04-12T23:44:25Z

src/nitpick/style.py

+        if token is not None and token.startswith("$"):
+            token = os.getenv(token[1:])


Maybe this works, I haven't tried.

Suggested change

if token is not None and token.startswith("$"):

token = os.getenv(token[1:])

if not token and token.startswith("$"):

token = os.getenv(token[1:])

andreoliwa · 2024-04-12T23:46:03Z

src/nitpick/style.py

+    @property
+    def authorization_header(self) -> dict[Literal["PRIVATE-TOKEN"], str] | None:
+        """Authorization header encoded in this URL."""
+        return {"PRIVATE-TOKEN": self.token} if self.token else None


This could a constant in constants.py:

GITLAB_TOKEN_KEY = "PRIVATE-TOKEN"

Or some similar name.

andreoliwa · 2024-04-12T23:48:03Z

src/nitpick/style.py

+            host=self.host,
+            path=[*self.project, "-", "raw", self.git_reference, *self.path],
+            query_params=self.query_params,
+        )


I reviewed this far, will continue later.

suhanoves · 2024-04-27T08:40:01Z

Thanks for the review!
I was on vacation, I’ll fix everything soon, according to your comments

jamesdow21 · 2024-10-24T14:54:18Z

Just recently discovered nitpick and would love to get it added to my own team's setup, but we are also on a self-hosted GitLab instance.

Is there still work being done on this by anyone? If not, I would be happy to try to make any changes to get it over the finish line.

suhanoves · 2024-10-24T17:09:48Z

Is there still work being done on this by anyone? If not, I would be happy to try to make any changes to get it over the finish line.

Hi, in fact, everything is ready here, we used this version on our project, you just need to add one test.
I think I even did this, but I'm afraid I won't find those fixes now.
Unfortunately, I suddenly changed the stack to another language and moved away from Python a little
See the comments on the tests from andreoliwa and add them to the merge request, please

suhanoves and others added 10 commits April 13, 2024 01:06

feat: implement GitLab package fetcher

f076859

docs: add GitLab package fetcher docs

0bde002

[pre-commit.ci] auto fixes from pre-commit.com hooks

215a17e

for more information, see https://pre-commit.ci

fix: fix ruff and mypy errors

21d4fa8

[pre-commit.ci] auto fixes from pre-commit.com hooks

fb6ecfd

for more information, see https://pre-commit.ci

fix: fix ruff error

e7be015

fix: fix docstring

7631b73

fix: fix type, error message and tests

a694463

fix: try to fix mypy error

d4ab58a

fix: fix type hinting for GitLabURL.query_params

e19728a

andreoliwa force-pushed the add_gitlab_fetcher branch from cadbe62 to e19728a Compare April 12, 2024 23:06

build: fix pylint warning temporarily so the CI job passes

0119a6c

andreoliwa reviewed Apr 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add GitLab fetcher #649

Add GitLab fetcher #649

suhanoves commented Mar 17, 2024

andreoliwa commented Mar 23, 2024

andreoliwa Apr 12, 2024

andreoliwa Apr 12, 2024

andreoliwa Apr 12, 2024

andreoliwa Apr 12, 2024

andreoliwa Apr 12, 2024

andreoliwa Apr 12, 2024

andreoliwa Apr 12, 2024

andreoliwa Apr 12, 2024

andreoliwa Apr 12, 2024

suhanoves commented Apr 27, 2024

jamesdow21 commented Oct 24, 2024

suhanoves commented Oct 24, 2024

		@@ -42,6 +42,9 @@ Remote style

		Use the URL of the remote file.

		Github

		@@ -1,3 +1,4 @@
		# pylint: disable=too-many-lines # TODO: refactor: break this into separate modules in a follow-up PR

		A literal token cannot start with a ``$``.
		All tokens must not contain any ``@`` or ``:`` characters.

		if token is not None and token.startswith("$"):
		token = os.getenv(token[1:])

Add GitLab fetcher #649

Are you sure you want to change the base?

Add GitLab fetcher #649

Conversation

suhanoves commented Mar 17, 2024

Proposed changes

Checklist

andreoliwa commented Mar 23, 2024

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

suhanoves commented Apr 27, 2024

jamesdow21 commented Oct 24, 2024

suhanoves commented Oct 24, 2024