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

fix(http): Use configured logger for retryClient #5040

Merged
merged 2 commits into from
Sep 11, 2024
Merged

Conversation

jliempt
Copy link
Member

@jliempt jliempt commented Sep 6, 2024

Changes

In the current implementation, a logger that is set through client.SetOptions() is overridden when retries are configured (which by default is the case). This only seems to be done to set logrus.DebugLevel.

The result of this is that the retryClient will always output some logs, and it's impossible to pass a logger with logger.SetOutput(io.Discard) that prevents these logs from being generated.

If no logger is explicitly configured for a retryClient (which is mostly the case), the verbose setting of the config.yml is already respected anyway, since log.Entry() is then used to set a logger.

  • Tests
  • Documentation

@jliempt jliempt requested a review from a team as a code owner September 6, 2024 10:18
CFenner

This comment was marked as duplicate.

Copy link
Member

@CCFenner CCFenner left a comment

Choose a reason for hiding this comment

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

👍

@CCFenner
Copy link
Member

CCFenner commented Sep 6, 2024

/it-go

1 similar comment
@jliempt
Copy link
Member Author

jliempt commented Sep 6, 2024

/it-go

@jliempt
Copy link
Member Author

jliempt commented Sep 11, 2024

/it-go

@jliempt jliempt enabled auto-merge (squash) September 11, 2024 09:31
Copy link

@jliempt jliempt merged commit 7e2604a into master Sep 11, 2024
12 checks passed
@jliempt jliempt deleted the jliempt/httpLoggerFix branch September 11, 2024 09:39
maxatsap added a commit to maxatsap/jenkins-library that referenced this pull request Oct 1, 2024
* origin/master: (35 commits)
  feat(notification): Notify in case of Release failure (SAP#5045)
  fix(docs): remove dead links (SAP#5051)
  chore: remove obsolete cloud sdk conversion file (SAP#5036)
  feat: fail if script is not found in package.json file (SAP#5029)
  feat(sonar): Enable trustengine for token retrieval (SAP#5046)
  feat(trustengine): Add new resource reference to parameter docs generation (SAP#5038)
  docs: add unit test tags flag (SAP#4947)
  CONTRIBUTING.md (SAP#5042)
  addon.yml may now contain wildCard MAXX (SAP#5039)
  fix(codeqlExecuteScan): handle spaces in path to maven settings file (SAP#5037)
  feat(trustengine): Integrate Trust Engine into step config resolver (SAP#5032)
  fix(http): Use configured logger for retryClient (SAP#5040)
  Updated helm.sh/helm from 13.14.0 to 13.14.2 (SAP#5041)
  Copy full project (SAP#5033)
  feat(vault): support complex data types in secrets (SAP#5006)
  Added pagination logic for retrieving projects from Black Duck server (SAP#5031)
  Update aws deps (SAP#5034)
  Add possible values and default (SAP#5030)
  Fix security issues reported by Black Duck (SAP#5014)
  Exposing build artifact metadata from maven and npm  (SAP#5008)
  ...
maxatsap added a commit to maxatsap/jenkins-library that referenced this pull request Oct 1, 2024
…fig-fix

* origin/master: (35 commits)
  feat(notification): Notify in case of Release failure (SAP#5045)
  fix(docs): remove dead links (SAP#5051)
  chore: remove obsolete cloud sdk conversion file (SAP#5036)
  feat: fail if script is not found in package.json file (SAP#5029)
  feat(sonar): Enable trustengine for token retrieval (SAP#5046)
  feat(trustengine): Add new resource reference to parameter docs generation (SAP#5038)
  docs: add unit test tags flag (SAP#4947)
  CONTRIBUTING.md (SAP#5042)
  addon.yml may now contain wildCard MAXX (SAP#5039)
  fix(codeqlExecuteScan): handle spaces in path to maven settings file (SAP#5037)
  feat(trustengine): Integrate Trust Engine into step config resolver (SAP#5032)
  fix(http): Use configured logger for retryClient (SAP#5040)
  Updated helm.sh/helm from 13.14.0 to 13.14.2 (SAP#5041)
  Copy full project (SAP#5033)
  feat(vault): support complex data types in secrets (SAP#5006)
  Added pagination logic for retrieving projects from Black Duck server (SAP#5031)
  Update aws deps (SAP#5034)
  Add possible values and default (SAP#5030)
  Fix security issues reported by Black Duck (SAP#5014)
  Exposing build artifact metadata from maven and npm  (SAP#5008)
  ...
maxatsap added a commit to maxatsap/jenkins-library that referenced this pull request Oct 1, 2024
…ix-fix

* origin/master: (35 commits)
  feat(notification): Notify in case of Release failure (SAP#5045)
  fix(docs): remove dead links (SAP#5051)
  chore: remove obsolete cloud sdk conversion file (SAP#5036)
  feat: fail if script is not found in package.json file (SAP#5029)
  feat(sonar): Enable trustengine for token retrieval (SAP#5046)
  feat(trustengine): Add new resource reference to parameter docs generation (SAP#5038)
  docs: add unit test tags flag (SAP#4947)
  CONTRIBUTING.md (SAP#5042)
  addon.yml may now contain wildCard MAXX (SAP#5039)
  fix(codeqlExecuteScan): handle spaces in path to maven settings file (SAP#5037)
  feat(trustengine): Integrate Trust Engine into step config resolver (SAP#5032)
  fix(http): Use configured logger for retryClient (SAP#5040)
  Updated helm.sh/helm from 13.14.0 to 13.14.2 (SAP#5041)
  Copy full project (SAP#5033)
  feat(vault): support complex data types in secrets (SAP#5006)
  Added pagination logic for retrieving projects from Black Duck server (SAP#5031)
  Update aws deps (SAP#5034)
  Add possible values and default (SAP#5030)
  Fix security issues reported by Black Duck (SAP#5014)
  Exposing build artifact metadata from maven and npm  (SAP#5008)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants