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.

Sign up for GitHub

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

Jump to bottom

Support Git clone over HTTP in Central Dogma. #954

Merged
merged 14 commits into from
May 22, 2024
Merged

Support Git clone over HTTP in Central Dogma. #954

merged 14 commits into from
May 22, 2024

Conversation

minwoox
Copy link
Member

@minwoox minwoox commented May 8, 2024 •
edited
Loading

Motivation:
Adding support for Git clone over HTTP in Central Dogma server would enhance its capabilities and make it more user-friendly. This PR partially addressed #543.
Please note that I didn't impelent Git over HTTP fully because:

  • We have APIs for commit separately.
  • The underlying Git repository could be changed to another format instead of Git.

Modifications:

  • Added support for Git clone over HTTP to Central Dogma.

Result:

  • You can now clone your repositories via Git clone over HTTP. 
    git clone -c http.extraHeader="Authorization: Bearer your-token" https://your-dogma.com/foo/bar.git

References:

The CLI command that allows you to view the packets being sent and received:

GIT_TRACE=2 GIT_CURL_VERBOSE=2 GIT_TRACE_PERFORMANCE=2 GIT_TRACE_PACK_ACCESS=2 GIT_TRACE_PACKET=2 GIT_TRACE_PACKFILE=2 GIT_TRACE_SETUP=2 GIT_TRACE_SHALLOW=2  git clone --depth=1 https://github.com/line/armeria.git

@minwoox
Support Git clone over HTTP in Central Dogma.
4af7e09 
Motivation:
Adding support for Git clone over HTTP in Central Dogma server would enhance its capabilities and make it more user-friendly.
This PR partially addressed line#543.
Please note that I didn't impelent Git over HTTP fully because
- We have APIs for commit separately.
- The underlying Git repository could be changed to other format instead of Git.
Modifications:
- Added support for Git clone over HTTP to Central Dogma.

Result:
You can now clone your repositories via Git clone over HTTP.
@minwoox minwoox requested review from ikhoon, jrhee17 and trustin as code owners May 8, 2024 07:09
trustin
trustin reviewed May 9, 2024
View reviewed changes
Copy link
Member

@trustin trustin left a comment

Choose a reason for hiding this comment

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

As a follow-up, could you also add an integration test that makes sure if this implementation works with Spring Cloud Config Server?

server/src/main/java/com/linecorp/centraldogma/server/internal/api/GitHttpService.java Show resolved Hide resolved
server/src/test/java/com/linecorp/centraldogma/server/internal/api/GitHttpServiceTest.java Outdated Show resolved Hide resolved
server/src/main/java/com/linecorp/centraldogma/server/internal/api/GitHttpService.java Outdated Show resolved Hide resolved
server/src/main/java/com/linecorp/centraldogma/server/internal/api/GitHttpService.java Outdated Show resolved Hide resolved
server/src/main/java/com/linecorp/centraldogma/server/internal/api/GitHttpService.java Outdated Show resolved Hide resolved
server/src/main/java/com/linecorp/centraldogma/server/internal/api/GitHttpService.java Outdated Show resolved Hide resolved
@minwoox minwoox mentioned this pull request May 9, 2024
Open
@minwoox
Copy link
Member Author

minwoox commented May 9, 2024

As a follow-up, could you also add an integration test that makes sure if this implementation works with Spring Cloud Config Server?

Thanks! Created. 😉
#955

@minwoox
Copy link
Member Author

minwoox commented May 9, 2024

@trustin, thanks! All addressed. 😉

@codecov Codecov
Copy link

codecov bot commented May 9, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 81.39535% with 16 lines in your changes are missing coverage. Please review.

Project coverage is 66.47%. Comparing base (aae194c) to head (f3079e9).
Report is 11 commits behind head on main.

❗ Current head f3079e9 differs from pull request most recent head 7c93f12. Consider uploading reports for the commit 7c93f12 to get more accurate results

Files Patch % Lines
...ntraldogma/server/internal/api/GitHttpService.java 84.21% 8 Missing and 4 partials ⚠️
...er/internal/storage/project/ProjectApiManager.java 33.33% 1 Missing and 1 partial ⚠️
...rnal/storage/repository/DefaultMetaRepository.java 0.00% 1 Missing ⚠️
...internal/storage/repository/RepositoryWrapper.java 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               main     #954      +/-   ##
============================================
- Coverage     66.86%   66.47%   -0.40%     
- Complexity     3529     3581      +52     
============================================
  Files           370      377       +7     
  Lines         14531    14859     +328     
  Branches       1563     1594      +31     
============================================
+ Hits           9716     9877     +161     
- Misses         3936     4097     +161     
- Partials        879      885       +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jrhee17
jrhee17 approved these changes May 10, 2024
View reviewed changes
Copy link
Contributor

@jrhee17 jrhee17 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 as a starting point to me 👍 Thanks @minwoox 👍 🙇 👍

server/src/main/java/com/linecorp/centraldogma/server/internal/api/GitHttpService.java Outdated Show resolved Hide resolved
server/src/main/java/com/linecorp/centraldogma/server/internal/api/GitHttpService.java Outdated Show resolved Hide resolved
...c/main/java/com/linecorp/centraldogma/server/internal/storage/project/ProjectApiManager.java Show resolved Hide resolved
@minwoox minwoox modified the milestones: 0.65.1, 0.66.0 May 10, 2024
ikhoon
ikhoon approved these changes May 10, 2024
View reviewed changes
Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

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

Fantastic feature! 🤩✨

@minwoox
Revert "Revert"
7c93f12 
This reverts commit 9e50f48.
minwoox added a commit to minwoox/armeria that referenced this pull request May 21, 2024
@minwoox
Add headers and MediaTypes for Git HTTP
e5b307c 
Motivation:
Added necessary headers and MediaTypes for Git HTTP, as a follow-up to line/centraldogma#954.
@minwoox minwoox mentioned this pull request May 21, 2024
Merged
trustin
trustin approved these changes May 22, 2024
View reviewed changes
Copy link
Member

@trustin trustin left a comment

Choose a reason for hiding this comment

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

Thanks!

minwoox added a commit to line/armeria that referenced this pull request May 22, 2024
@minwoox
Add headers and MediaTypes for Git HTTP (#5689)
a68b5f9 
Motivation:
Added necessary headers and MediaTypes for Git HTTP, as a follow-up to line/centraldogma#954.
@minwoox minwoox merged commit 5463d22 into line:main May 22, 2024
9 of 10 checks passed
@minwoox minwoox deleted the git_clone_http branch May 22, 2024 10:44
Dogacel pushed a commit to Dogacel/armeria that referenced this pull request Jun 8, 2024
@minwoox @Dogacel
Add headers and MediaTypes for Git HTTP (line#5689)
b820149 
Motivation:
Added necessary headers and MediaTypes for Git HTTP, as a follow-up to line/centraldogma#954.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrhee17 jrhee17 jrhee17 approved these changes

@ikhoon ikhoon ikhoon approved these changes

@trustin trustin trustin approved these changes

Assignees
No one assigned
Labels
new feature
Projects
None yet
Milestone
0.66.0
Development

Successfully merging this pull request may close these issues.
4 participants
@minwoox @trustin @ikhoon @jrhee17