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

fix(artifacts): use env https_proxy as oss client option proxy_host. Fixes #12313 #12383

Merged
merged 2 commits into from
Sep 10, 2024
Merged

fix(artifacts): use env https_proxy as oss client option proxy_host. Fixes #12313 #12383

merged 2 commits into from
Sep 10, 2024

Conversation

jingkkkkai
Copy link
Contributor

Fixes #12313

Motivation

Modifications

Verification

Beyond this PR

@jingkkkkai jingkkkkai changed the title feat: use env https_proxy as oss client option proxy_host #12313 chore: use env https_proxy as oss client option proxy_host #12313 Dec 18, 2023
@jingkkkkai jingkkkkai mentioned this pull request Dec 20, 2023
Closed
3 tasks
terrytangyuan
terrytangyuan reviewed Dec 20, 2023
View reviewed changes
Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

@shuangkun Could you help review this?

@shuangkun
Copy link
Member

shuangkun commented Dec 20, 2023 •
edited by agilgur5
Loading

Okay

@agilgur5 agilgur5 added the area/artifacts S3/GCP/OSS/Git/HDFS etc label Jan 27, 2024
@isubasinghe isubasinghe mentioned this pull request Jan 30, 2024
Closed
7 tasks
@jingkkkkai jingkkkkai force-pushed the add-proxy-host-for-oss-client branch from b479caa to f00768f Compare August 20, 2024 01:43
@carolkao
Copy link

Hi @terrytangyuan and @isubasinghe,
I'd like to expedite the progress of this PR. It's crucial for supporting proxy settings in OSS client for us. Currently, we rely on Argo v3.4.8 with our self-fix build in our production environment. However, this setup is blocking any of our Argo Workflow upgrade plans, including trying out the new build which addresses the potential issue detailed in #11948.
I hope you can help move the review forward. Thank you so much.

@shuangkun
Copy link
Member

Could you solve the test error？

@carolkao
Copy link

carolkao commented Aug 29, 2024 •
edited
Loading

Hi @jingkkkkai , please help to solve the error for this PR.

@isubasinghe
Copy link
Member

@jingkkkkai DCO isn't passing and the test suite isn't passing, maybe try commit an empty commit.

@jingkkkkai jingkkkkai force-pushed the add-proxy-host-for-oss-client branch from f00768f to 9d85eaa Compare September 2, 2024 03:47
@jingkkkkai jingkkkkai force-pushed the add-proxy-host-for-oss-client branch from 9d85eaa to 6782685 Compare September 2, 2024 07:27
@jingkkkkai
Copy link
Contributor Author

jingkkkkai commented Sep 2, 2024 •
edited
Loading

Hi @shuangkun , @isubasinghe , @terrytangyuan

the test suite passed after adding an empty commit, thanks for your help
then could you please help review the pr again?

additionally, is there a way to apply this change to branch v3.4?

@carolkao
Copy link

carolkao commented Sep 9, 2024

Hi @isubasinghe,
All checks have passed now, can you help to move this PR forward?

isubasinghe
isubasinghe approved these changes Sep 9, 2024
View reviewed changes
Copy link
Member

@isubasinghe isubasinghe left a comment

Choose a reason for hiding this comment

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

LGTM

@carolkao
Copy link

Hi @terrytangyuan ,

Do you think Argo's development team could help cherry-pick this fix into the 3.4 branch? We're struggling with the build environment, and I believe it would be much easier for Argo's team to handle this.

Thanks,
Carol

@terrytangyuan terrytangyuan merged commit 7fee019 into argoproj:main Sep 10, 2024
27 checks passed
@terrytangyuan terrytangyuan mentioned this pull request Sep 10, 2024
Open
@terrytangyuan
Copy link
Member

@carolkao See #11851 (comment)

@agilgur5 agilgur5 changed the title chore: use env https_proxy as oss client option proxy_host #12313 fix(artifacts): use env https_proxy as oss client option proxy_host. Fixes #12313 Sep 10, 2024
@carolkao
Copy link

@carolkao See #11851 (comment)

Cool, thank you @terrytangyuan . Looking forward to the next 3.4 release.

Joibel pushed a commit to pipekit/argo-workflows that referenced this pull request Sep 19, 2024
@jingkkkkai @Joibel
chore: use env https_proxy as oss client option proxy_host argoproj#1…
6d06f3a 
…2313 (argoproj#12383)

Signed-off-by: Jingkai Hung <[email protected]>
@Joibel Joibel mentioned this pull request Sep 20, 2024
Open
Joibel pushed a commit that referenced this pull request Sep 20, 2024
@jingkkkkai @Joibel
chore: use env https_proxy as oss client option proxy_host #12313 (#1…
2f60d15 
…2383)

Signed-off-by: Jingkai Hung <[email protected]>
@agilgur5 agilgur5 added this to the v3.5.x patches milestone Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@isubasinghe isubasinghe isubasinghe approved these changes

@terrytangyuan terrytangyuan terrytangyuan approved these changes

Assignees
No one assigned
Labels
area/artifacts S3/GCP/OSS/Git/HDFS etc
Projects
None yet
Milestone
v3.5.x patches
Development

Successfully merging this pull request may close these issues.

env http_proxy in wait/init container does not work for oss client
6 participants
@jingkkkkai @shuangkun @carolkao @isubasinghe @terrytangyuan @agilgur5