Skip to content

Commit 42d683c

Browse files
feat(#34): support self-managed GitLab (#145)
* feat(#34): support self-hosted GitLab * fix: apply suggestions from lgtm code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix: syntax and add test --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent ec42801 commit 42d683c

File tree

12 files changed

+115
-50
lines changed

12 files changed

+115
-50
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ There are three available severities for comments:
143143

144144
lgtm aims to work with as many services as possible, and that includes remote repository providers. At the moment, lgtm supports:
145145

146-
- [GitLab](https://gitlab.com) (only gitlab.com, not self-hosted)
146+
- [GitLab](https://gitlab.com) (both gitlab.com and [self-managed](https://about.gitlab.com/install/)).
147147
- [GitHub](https://github.com)
148148

149149
lgtm will autodetect the url of the pull request passed as an argument.

src/lgtm_ai/__main__.py

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from collections.abc import Callable
44
from importlib.metadata import version
55
from typing import Any, assert_never, get_args
6+
from urllib.parse import urlparse
67

78
import click
89
import httpx
@@ -179,7 +180,12 @@ def review(target: PRUrl | LocalRepository, config: str | None, verbose: int, **
179180
formatter: Formatter[Any] = MarkDownFormatter(
180181
add_ranges_to_suggestions=git_source_supports_multiline_suggestions(target.source)
181182
)
182-
git_client = get_git_client(source=target.source, token=resolved_config.git_api_key, formatter=formatter)
183+
git_client = get_git_client(
184+
source=target.source,
185+
token=resolved_config.git_api_key,
186+
formatter=formatter,
187+
url=target.base_url if isinstance(target, PRUrl) else None,
188+
)
183189
issues_client = _get_issues_client(resolved_config, git_client, formatter)
184190

185191
code_reviewer = CodeReviewer(
@@ -235,7 +241,9 @@ def guide(
235241
config_file=config,
236242
).resolve_config(target)
237243
agent_extra_settings = AgentSettings(retries=resolved_config.ai_retries)
238-
git_client = get_git_client(source=target.source, token=resolved_config.git_api_key, formatter=MarkDownFormatter())
244+
git_client = get_git_client(
245+
source=target.source, token=resolved_config.git_api_key, formatter=MarkDownFormatter(), url=target.base_url
246+
)
239247
review_guide = ReviewGuideGenerator(
240248
guide_agent=get_guide_agent_with_settings(agent_extra_settings),
241249
model=get_ai_model(
@@ -283,20 +291,23 @@ def _get_formatter_and_printer(output_format: OutputFormat) -> tuple[Formatter[A
283291
def _get_issues_client(
284292
resolved_config: ResolvedConfig, git_client: GitClient | None, formatter: Formatter[Any]
285293
) -> IssuesClient | None:
286-
"""Select a different issues client for retrieving issues.
294+
"""Get the issues client based on the resolved configuration.
287295
288-
Will only return a different client if all of the following are true:
289-
1) Be used at all
290-
2) Be retrieved from a git platform and not elsewhere (e.g., Jira, Asana, etc.)
291-
3) Have a specific API key configured
296+
It can be a GitClient for GitHub/GitLab issues, or a Jira client.
297+
If issues are not configured with a specific platform, it will fall back
298+
to using the main `git_client`.
292299
"""
293300
issues_client: IssuesClient | None = git_client
294301
if not resolved_config.issues_url or not resolved_config.issues_platform or not resolved_config.issues_regex:
295302
return issues_client
296303
if resolved_config.issues_platform.is_git_platform:
297304
if resolved_config.issues_api_key:
305+
parsed_issues_url = urlparse(str(resolved_config.issues_url))
298306
issues_client = get_git_client(
299-
source=resolved_config.issues_platform, token=resolved_config.issues_api_key, formatter=formatter
307+
source=resolved_config.issues_platform,
308+
token=resolved_config.issues_api_key,
309+
formatter=formatter,
310+
url=f"{parsed_issues_url.scheme}://{parsed_issues_url.netloc}",
300311
)
301312
elif resolved_config.issues_platform == IssuesPlatform.jira:
302313
if not resolved_config.issues_api_key or not resolved_config.issues_user:

src/lgtm_ai/base/schemas.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ def is_git_platform(self) -> bool:
2626
@dataclass(frozen=True, slots=True)
2727
class PRUrl:
2828
full_url: str
29+
base_url: str
2930
repo_path: str
3031
pr_number: int
3132
source: PRSource

src/lgtm_ai/git_client/utils.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,16 @@
77
from lgtm_ai.git_client.gitlab import GitlabClient
88

99

10-
def get_git_client(source: PRSource | IssuesPlatform, token: str, formatter: Formatter[str]) -> GitClient | None:
10+
def get_git_client(
11+
source: PRSource | IssuesPlatform, token: str, formatter: Formatter[str], url: str | None = None
12+
) -> GitClient | None:
1113
"""Return a GitClient instance based on the provided PR URL."""
1214
git_client: GitClient
1315

1416
if source == "gitlab":
15-
git_client = GitlabClient(gitlab.Gitlab(private_token=token), formatter=formatter)
17+
git_client = GitlabClient(gitlab.Gitlab(url=url, private_token=token), formatter=formatter)
1618
elif source == "github":
19+
# TODO: Handle GitHub Enterprise with a custom URL
1720
git_client = GitHubClient(github.Github(login_or_token=token), formatter=formatter)
1821
elif source == "local":
1922
return None

src/lgtm_ai/validators.py

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,6 @@ def parse_target(ctx: click.Context, param: str, value: object) -> PRUrl | Local
4242
)
4343

4444
match parsed.netloc:
45-
case AllowedLocations.Gitlab:
46-
return _parse_pr_url(
47-
parsed,
48-
split_str="/-/merge_requests/",
49-
source=PRSource.gitlab,
50-
error_url_msg="The PR URL must be a merge request URL.",
51-
error_num_msg="The PR URL must contain a valid MR number.",
52-
)
53-
5445
case AllowedLocations.Github:
5546
return _parse_pr_url(
5647
parsed,
@@ -59,10 +50,22 @@ def parse_target(ctx: click.Context, param: str, value: object) -> PRUrl | Local
5950
error_url_msg="The PR URL must be a pull request URL.",
6051
error_num_msg="The PR URL must contain a valid PR number.",
6152
)
62-
6353
case _:
64-
raise click.BadParameter(
65-
f"The PR URL host must be one of: {', '.join([s.value for s in AllowedLocations.__members__.values()])}"
54+
# We support for GitLab cloud (.com) and self-hosted (custom domain)
55+
# TODO: When we support more git providers with custom urls, we need to revisit this and
56+
# probably add a `--git-platform` option to the CLI.
57+
if "/-/merge_requests/" not in parsed.path:
58+
raise click.BadParameter(
59+
f"The PR URL host '{parsed.netloc}' is not supported. "
60+
"lgtm-ai currently supports github.com and GitLab (cloud or self-hosted). "
61+
"GitLab merge request URLs must contain '/-/merge_requests/'."
62+
)
63+
return _parse_pr_url(
64+
parsed,
65+
split_str="/-/merge_requests/",
66+
source=PRSource.gitlab,
67+
error_url_msg="The PR URL must be a merge request URL.",
68+
error_num_msg="The PR URL must contain a valid MR number.",
6669
)
6770

6871

@@ -135,6 +138,7 @@ def _parse_pr_url(
135138

136139
return PRUrl(
137140
full_url=parsed.geturl(),
141+
base_url=f"{parsed.scheme}://{parsed.netloc}",
138142
repo_path=project_path.strip("/"),
139143
pr_number=pr_num,
140144
source=source,

tests/config/test_handler.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
target = PRUrl(
1616
full_url="https://gitlab.com/user/repo/-/merge_requests/1",
17+
base_url="https://gitlab.com",
1718
repo_path="user/repo",
1819
pr_number=1,
1920
source=PRSource.gitlab,

tests/git_client/test_github.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
MockGithubUrl = PRUrl(
2929
full_url="https://github.com/foo/bar/pull/1",
3030
repo_path="foo/bar",
31+
base_url="https://github.com",
3132
pr_number=1,
3233
source=PRSource.github,
3334
)
@@ -234,7 +235,7 @@ def test_get_file_contents_single_file() -> None:
234235
mock.Mock(decoded_content=b"lorem ipsum dolor sit amet"),
235236
]
236237
content = client.get_file_contents(
237-
PRUrl(full_url="https://foo", repo_path="path", pr_number=1, source=PRSource.github),
238+
PRUrl(full_url="https://foo", base_url="https://foo", repo_path="path", pr_number=1, source=PRSource.github),
238239
file_path="important.py",
239240
branch_name="source",
240241
)
@@ -263,7 +264,7 @@ def test_get_file_contents_single_file_multiple_objects() -> None:
263264
]
264265
]
265266
content = client.get_file_contents(
266-
PRUrl(full_url="https://foo", repo_path="path", pr_number=1, source=PRSource.github),
267+
PRUrl(full_url="https://foo", base_url="https://foo", repo_path="path", pr_number=1, source=PRSource.github),
267268
file_path="important.py",
268269
branch_name="source",
269270
)
@@ -303,12 +304,12 @@ def test_get_file_contents_one_file_missing() -> None:
303304
github.GithubException(status=404, message="Not Found"), # target branch
304305
]
305306
content_1 = client.get_file_contents(
306-
PRUrl(full_url="https://foo", repo_path="path", pr_number=1, source=PRSource.github),
307+
PRUrl(full_url="https://foo", base_url="https://foo", repo_path="path", pr_number=1, source=PRSource.github),
307308
file_path="important.py",
308309
branch_name="source",
309310
)
310311
content_2 = client.get_file_contents(
311-
PRUrl(full_url="https://foo", repo_path="path", pr_number=1, source=PRSource.github),
312+
PRUrl(full_url="https://foo", base_url="https://foo", repo_path="path", pr_number=1, source=PRSource.github),
312313
file_path="logic.py",
313314
branch_name="source",
314315
)
@@ -356,7 +357,7 @@ def test_post_review_successful() -> None:
356357
)
357358

358359
client.publish_review(
359-
PRUrl(full_url="https://foo", repo_path="path", pr_number=1, source=PRSource.github),
360+
PRUrl(full_url="https://foo", base_url="https://foo", repo_path="path", pr_number=1, source=PRSource.github),
360361
fake_review,
361362
)
362363

@@ -430,7 +431,7 @@ def test_post_review_fallback_to_single_line() -> None:
430431
]
431432

432433
client.publish_review(
433-
PRUrl(full_url="https://foo", repo_path="path", pr_number=1, source=PRSource.github),
434+
PRUrl(full_url="https://foo", base_url="https://foo", repo_path="path", pr_number=1, source=PRSource.github),
434435
fake_review,
435436
)
436437

@@ -487,7 +488,8 @@ def test_publish_guide_successful() -> None:
487488
client = mock_github_client(m_repo)
488489

489490
client.publish_guide(
490-
PRUrl(full_url="https://foo", repo_path="path", pr_number=1, source=PRSource.github), FAKE_GUIDE
491+
PRUrl(full_url="https://foo", base_url="https://foo", repo_path="path", pr_number=1, source=PRSource.github),
492+
FAKE_GUIDE,
491493
)
492494

493495
assert m_pr.create_review.call_args_list == [

tests/git_client/test_gitlab.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
MockGitlabUrl = PRUrl(
2626
full_url="https://gitlab.com/foo/-/merge_requests/1",
27+
base_url="https://gitlab.com",
2728
repo_path="foo",
2829
pr_number=1,
2930
source=PRSource.gitlab,
@@ -328,12 +329,12 @@ def test_get_file_contents_multiple_files() -> None:
328329
client = mock_gitlab_client(m_project)
329330

330331
contents_1 = client.get_file_contents(
331-
PRUrl(full_url="https://foo", repo_path="path", pr_number=1, source=PRSource.gitlab),
332+
PRUrl(full_url="https://foo", base_url="https://foo", repo_path="path", pr_number=1, source=PRSource.gitlab),
332333
file_path="important.py",
333334
branch_name="source",
334335
)
335336
contents_2 = client.get_file_contents(
336-
PRUrl(full_url="https://foo", repo_path="path", pr_number=1, source=PRSource.gitlab),
337+
PRUrl(full_url="https://foo", base_url="https://foo", repo_path="path", pr_number=1, source=PRSource.gitlab),
337338
file_path="logic.py",
338339
branch_name="source",
339340
)
@@ -354,19 +355,19 @@ def test_get_file_contents_one_file_missing() -> None:
354355
client = mock_gitlab_client(m_project)
355356

356357
missing_1 = client.get_file_contents(
357-
PRUrl(full_url="https://foo", repo_path="path", pr_number=1, source=PRSource.gitlab),
358+
PRUrl(full_url="https://foo", base_url="https://foo", repo_path="path", pr_number=1, source=PRSource.gitlab),
358359
file_path="whatever",
359360
branch_name="source",
360361
)
361362

362363
missing_2 = client.get_file_contents(
363-
PRUrl(full_url="https://foo", repo_path="path", pr_number=1, source=PRSource.gitlab),
364+
PRUrl(full_url="https://foo", base_url="https://foo", repo_path="path", pr_number=1, source=PRSource.gitlab),
364365
file_path="whatever",
365366
branch_name="target",
366367
)
367368

368369
contents = client.get_file_contents(
369-
PRUrl(full_url="https://foo", repo_path="path", pr_number=1, source=PRSource.gitlab),
370+
PRUrl(full_url="https://foo", base_url="https://foo", repo_path="path", pr_number=1, source=PRSource.gitlab),
370371
file_path="logic.py",
371372
branch_name="source",
372373
)

tests/review/test_context.py

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@ def test_retrieve_additional_context_from_git(self) -> None:
2121
)
2222

2323
pr_url = PRUrl(
24-
full_url="https://example.com/repo/pull/1", repo_path="repo", pr_number=1, source=PRSource.github
24+
full_url="https://example.com/repo/pull/1",
25+
base_url="https://example.com",
26+
repo_path="repo",
27+
pr_number=1,
28+
source=PRSource.github,
2529
)
2630

2731
additional_context = context_retriever.get_additional_context(
@@ -50,7 +54,11 @@ def test_retrieve_additional_context_from_url(self) -> None:
5054
)
5155

5256
pr_url = PRUrl(
53-
full_url="https://example.com/repo/pull/1", repo_path="repo", pr_number=1, source=PRSource.github
57+
full_url="https://example.com/repo/pull/1",
58+
base_url="https://example.com",
59+
repo_path="repo",
60+
pr_number=1,
61+
source=PRSource.github,
5462
)
5563

5664
additional_context = context_retriever.get_additional_context(
@@ -73,7 +81,11 @@ def test_retrieve_additional_context_from_config(self) -> None:
7381
git_client=MockGitClient(), issues_client=MockGitClient(), httpx_client=mock.Mock(spec=httpx.Client)
7482
)
7583
pr_url = PRUrl(
76-
full_url="https://example.com/repo/pull/1", repo_path="repo", pr_number=1, source=PRSource.github
84+
full_url="https://example.com/repo/pull/1",
85+
base_url="https://example.com",
86+
repo_path="repo",
87+
pr_number=1,
88+
source=PRSource.github,
7789
)
7890

7991
additional_context = context_retriever.get_additional_context(
@@ -108,7 +120,13 @@ def test_get_context_multiple_files(self, client: GitClient) -> None:
108120
)
109121

110122
context = context_retriever.get_code_context(
111-
PRUrl(full_url="https://foo", repo_path="path", pr_number=1, source=PRSource.github),
123+
PRUrl(
124+
full_url="https://foo",
125+
base_url="https://example.com",
126+
repo_path="path",
127+
pr_number=1,
128+
source=PRSource.github,
129+
),
112130
pr_diff=pr_diff,
113131
)
114132

@@ -139,7 +157,13 @@ def test_get_context_one_file_missing(self, client: GitClient) -> None:
139157
)
140158

141159
context = context_retriever.get_code_context(
142-
PRUrl(full_url="https://foo", repo_path="path", pr_number=1, source=PRSource.gitlab),
160+
PRUrl(
161+
full_url="https://foo",
162+
base_url="https://example.com",
163+
repo_path="path",
164+
pr_number=1,
165+
source=PRSource.gitlab,
166+
),
143167
pr_diff=pr_diff,
144168
)
145169

tests/review/test_guide.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ def test_get_guide_from_url_valid() -> None:
3232
config=ResolvedConfig(ai_api_key="", git_api_key=""),
3333
)
3434
guide = guide_generator.generate_review_guide(
35-
pr_url=PRUrl(full_url="foo", repo_path="foo", pr_number=1, source=PRSource.gitlab)
35+
pr_url=PRUrl(full_url="foo", base_url="foo", repo_path="foo", pr_number=1, source=PRSource.gitlab)
3636
)
3737

3838
assert guide == ReviewGuide(

0 commit comments

Comments
 (0)