Skip to content

refactor(cli): remove capture command global logger#2215

Closed
Kelvinoppong wants to merge 2 commits into
microsoft:mainfrom
Kelvinoppong:issue-585-remove-cli-global-logger
Closed

refactor(cli): remove capture command global logger#2215
Kelvinoppong wants to merge 2 commits into
microsoft:mainfrom
Kelvinoppong:issue-585-remove-cli-global-logger

Conversation

@Kelvinoppong

@Kelvinoppong Kelvinoppong commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Description

This pull request removes the CLI capture flow's dependency on the global retinacmd.Logger and replaces it with explicit logger injection.

The main changes are:

  • initialize the CLI logger in cli/main.go
  • pass a scoped logger into capture.NewCommand
  • thread the logger through the capture create, delete, and download command paths
  • update download helpers and DownloadService to use the injected logger instead of package-global state
  • remove the old global CLI logger from cli/cmd/root.go
  • update tests to construct and pass a test logger explicitly
  • add a fail-fast validation for blob URLs without a SAS token so blob validation tests do not hang

This keeps the capture command behavior the same while making the logger usage more explicit and easier to test.

Related Issue

Related to #585

Checklist

  • I have read the contributing documentation.
  • I signed and signed-off the commits (git commit -S -s ...). See this documentation on signing commits.
  • I have correctly attributed the author(s) of the code.
  • I have tested the changes locally.
  • I have followed the project's style guidelines.
  • I have updated the documentation, if necessary.
  • I have added tests, if applicable.

Screenshots (if applicable) or Testing Completed

Tested locally with:

  • go test -v ./cli/cmd/capture/...
  • go test -v ./cli/...
  • go test -run '^$' ./...
  • go test -v -run 'TestDownloadFromBlobValidation' -count=1 ./cli/cmd/capture

Additional Notes

This change is intentionally scoped to the CLI capture flow. It does not attempt to remove logger singletons from the rest of the codebase.

The blob download validation now returns early when the blob URL does not include a SAS token with read/list permissions. This makes the related unit test deterministic in local environments.

Signed-off-by: Kelvin <oppongk139@gmail.com>
@Kelvinoppong Kelvinoppong requested a review from a team as a code owner April 20, 2026 22:34
@Kelvinoppong

Copy link
Copy Markdown
Contributor Author

@nddq, @nairashu this PR is ready for review

@nddq

nddq commented Apr 23, 2026

Copy link
Copy Markdown
Member

This PR is a duplicated of #2144, please make sure next time the issue you are taking on don't have an active PR addressing it to avoid duplicating work, closing this for now, thanks!

@nddq nddq closed this Apr 23, 2026
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.

2 participants