Skip to content

feat: make git requests configurable #15646

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

Merged
merged 3 commits into from
Oct 24, 2023

Conversation

alexmt
Copy link
Collaborator

@alexmt alexmt commented Sep 24, 2023

PR introduces an ARGOCD_GIT_REQUEST_TIMEOUT env variable that allows configuring git request timeouts (currently 15s by default)

@alexmt alexmt requested a review from a team as a code owner September 24, 2023 15:37
Copy link
Member

@ishitasequeira ishitasequeira 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!! Just one request to add some documentation.

@alexmt alexmt force-pushed the feat/configurable-git-client-timeout branch from 1f08e30 to e5a6aee Compare September 24, 2023 15:45
@codecov
Copy link

codecov bot commented Sep 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (56a7bb7) 49.53% compared to head (8e4021a) 49.54%.
Report is 6 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #15646      +/-   ##
==========================================
+ Coverage   49.53%   49.54%   +0.01%     
==========================================
  Files         269      269              
  Lines       46622    46641      +19     
==========================================
+ Hits        23095    23110      +15     
- Misses      21255    21258       +3     
- Partials     2272     2273       +1     
Files Coverage Δ
util/git/client.go 52.25% <100.00%> (ø)

... and 4 files with indirect coverage changes

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

@crenshaw-dev
Copy link
Member

Should this be configured via a CLI arg/env var default on the repo-server command? Would improve discoverability.

@alexmt alexmt force-pushed the feat/configurable-git-client-timeout branch from e5a6aee to 61384b5 Compare October 23, 2023 19:14

Verified

This commit was signed with the committer’s verified signature.
alexmt Alexander Matyushentsev
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
@alexmt alexmt force-pushed the feat/configurable-git-client-timeout branch from 61384b5 to f40f4f8 Compare October 23, 2023 19:20
@alexmt alexmt requested a review from a team as a code owner October 23, 2023 23:23
@alexmt
Copy link
Collaborator Author

alexmt commented Oct 23, 2023

Thank you for review @crenshaw-dev and @ishitasequeira !

I've kept working on debugging the issue caused by the slow git provider and realized another setting is required: ARGOCD_GIT_LS_REMOTE_PARALLELISM_LIMIT . Together with ARGOCD_GIT_REQUEST_TIMEOUT, these settings helps to address issues caused by overloaded git provider. I've documented both in argocd-cmd-params-cm configmap.

PTAL

Verified

This commit was signed with the committer’s verified signature.
alexmt Alexander Matyushentsev
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
@alexmt alexmt force-pushed the feat/configurable-git-client-timeout branch from 97ed3f6 to 47c15cd Compare October 24, 2023 14:50
@alexmt alexmt requested a review from ishitasequeira October 24, 2023 14:59

Verified

This commit was signed with the committer’s verified signature.
alexmt Alexander Matyushentsev
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
Copy link
Member

@ishitasequeira ishitasequeira left a comment

Choose a reason for hiding this comment

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

LGTM!!

@alexmt alexmt enabled auto-merge (squash) October 24, 2023 19:55
@alexmt alexmt merged commit f37d24f into argoproj:master Oct 24, 2023
@alexmt alexmt deleted the feat/configurable-git-client-timeout branch October 24, 2023 20:59
ymktmk pushed a commit to ymktmk/argo-cd that referenced this pull request Oct 29, 2023
* feat: make git requests configurable

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* docs: mention new settings in 'argocd-cmd-params-cm' configmap

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* add comment about ignored error

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

---------

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
jmilic1 pushed a commit to jmilic1/argo-cd that referenced this pull request Nov 13, 2023
* feat: make git requests configurable

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* docs: mention new settings in 'argocd-cmd-params-cm' configmap

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* add comment about ignored error

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

---------

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
git rebase HEAD~73 --signoff
vladfr pushed a commit to vladfr/argo-cd that referenced this pull request Dec 13, 2023

Verified

This commit was signed with the committer’s verified signature.
vladfr Vlad Fratila
* feat: make git requests configurable

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* docs: mention new settings in 'argocd-cmd-params-cm' configmap

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* add comment about ignored error

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

---------

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
tesla59 pushed a commit to tesla59/argo-cd that referenced this pull request Dec 16, 2023
* feat: make git requests configurable

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* docs: mention new settings in 'argocd-cmd-params-cm' configmap

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* add comment about ignored error

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

---------

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
alexmt added a commit to alexmt/argo-cd that referenced this pull request Jan 19, 2024
* feat: make git requests configurable

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* docs: mention new settings in 'argocd-cmd-params-cm' configmap

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* add comment about ignored error

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

---------

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
Hariharasuthan99 pushed a commit to AmadeusITGroup/argo-cd that referenced this pull request Jun 16, 2024
* feat: make git requests configurable

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* docs: mention new settings in 'argocd-cmd-params-cm' configmap

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* add comment about ignored error

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

---------

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants