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.

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

Support Git clone over HTTP in Central Dogma. #954

Merged
merged 14 commits into from
May 22, 2024

Conversation

minwoox
Copy link
Member

@minwoox minwoox commented May 8, 2024

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

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.
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?

@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. 😉

Copy link

codecov bot commented May 9, 2024

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.

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 👍 🙇 👍

@minwoox minwoox modified the milestones: 0.65.1, 0.66.0 May 10, 2024
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! 🤩✨

This reverts commit 9e50f48.
minwoox added a commit to minwoox/armeria that referenced this pull request May 21, 2024
Motivation:
Added necessary headers and MediaTypes for Git HTTP, as a follow-up to line/centraldogma#954.
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
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
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants