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

Migration will clone repository from the proxy #16697

Closed
wants to merge 1 commit into from

Conversation

idealism-xxm
Copy link

@idealism-xxm idealism-xxm commented Aug 15, 2021

Fix: #12018

Signed-off-by: idealism-xxm biaochekuangmo@yeah.net

@techknowlogick
Copy link
Member

Thanks for this PR, my understanding of golang was that it picks up proxy settings via the HTTP_PROXY environment variable?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 15, 2021
@idealism-xxm
Copy link
Author

The http.DefaultTransport will use proxy from environment, and it's only used when there is no custom transport.

Gitea uses a custom transport without proxy in file modules/migrations/gogs.go and modules/migrations/github.go, so we need to add a proxy.

Maybe we can only use proxy from the environment?

@codecov-commenter
Copy link

codecov-commenter commented Aug 15, 2021

Codecov Report

Merging #16697 (072e8c2) into main (ca13e1d) will increase coverage by 0.04%.
The diff coverage is 48.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #16697      +/-   ##
==========================================
+ Coverage   45.36%   45.40%   +0.04%     
==========================================
  Files         758      758              
  Lines       85120    85306     +186     
==========================================
+ Hits        38612    38732     +120     
- Misses      40245    40294      +49     
- Partials     6263     6280      +17     
Impacted Files Coverage Δ
models/access.go 71.34% <0.00%> (ø)
models/models.go 53.42% <ø> (ø)
models/org_team.go 55.19% <ø> (-0.01%) ⬇️
models/repo_collaboration.go 58.62% <0.00%> (-0.82%) ⬇️
models/repo_generate.go 15.00% <0.00%> (ø)
models/repo_transfer.go 43.47% <0.00%> (ø)
models/ssh_key.go 50.96% <0.00%> (-0.60%) ⬇️
models/token.go 66.66% <0.00%> (-5.75%) ⬇️
models/user.go 54.40% <0.00%> (-0.13%) ⬇️
modules/context/org.go 37.73% <0.00%> (ø)
... and 87 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6bf5afe...072e8c2. Read the comment docs.

@6543 6543 added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Aug 15, 2021
@lafriks
Copy link
Member

lafriks commented Aug 16, 2021

I think we should reuse HTTP_PROXY env and not introduce new config settings for this

@idealism-xxm
Copy link
Author

I think we should reuse HTTP_PROXY env and not introduce new config settings for this

Ok, I agree with you and I'll fix this.

@idealism-xxm idealism-xxm changed the title Add proxy setting Migration will clone repository from the proxy Aug 16, 2021
@lunny
Copy link
Member

lunny commented Aug 16, 2021

I think we should reuse HTTP_PROXY env and not introduce new config settings for this

I don't think so. Assume we want to add proxy for migrating from github but not another gitea instance, how should we do? We should have a global proxy i.e. #15234 and then set the proxy per domain.

@idealism-xxm
Copy link
Author

I think we should reuse HTTP_PROXY env and not introduce new config settings for this

I don't think so. Assume we want to add proxy for migrating from github but not another gitea instance, how should we do? We should have a global proxy i.e. #15234 and then set the proxy per domain.

Alright. I thought we could set these rules in proxy software.

@lunny
Copy link
Member

lunny commented Aug 18, 2021

Since #16704 has been merged, I think we can close this one. @idealism-xxm You can also send another PR to improve the proxy feature.

@6543
Copy link
Member

6543 commented Aug 18, 2021

-> #16704 (comment)

@6543 6543 closed this Aug 18, 2021
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migration does not clone repository from the proxy
7 participants