Skip to content

fix: do not print host_base to stdout#530

Merged
MalinAhlberg merged 2 commits intomainfrom
fix/print-hostbase-to-stderr
Mar 28, 2025
Merged

fix: do not print host_base to stdout#530
MalinAhlberg merged 2 commits intomainfrom
fix/print-hostbase-to-stderr

Conversation

@MalinAhlberg
Copy link
Copy Markdown
Member

Related issue(s) and PR(s)
This PR closes #525 .

Description

  • Information about host base is now printed to stderr.
  • Tests are updated
  • The actual printing function is moved to helpers, to simplify the process in case we want to update the message in the future.

How to test
For example, run

$ sda-cli -config testing/s3cmd.conf upload go.sum

And make sure the info about the host base is in stderr.
Also test that counting the number of files works:

sda-cli -config s3cfg list  | wc -l
The provided access token expires in 12 hours and 24 minutes.
Consider renewing the token.
Remote server (host_base): localhost:18000
1

And similarly for download.

@MalinAhlberg MalinAhlberg requested a review from a team March 27, 2025 12:41
Copy link
Copy Markdown
Contributor

@jbygdell jbygdell left a comment

Choose a reason for hiding this comment

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

Two unnecessary blank lines but else fine.

Comment thread helpers/helpers.go
Copy link
Copy Markdown
Contributor

@nanjiangshu nanjiangshu 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!

@MalinAhlberg MalinAhlberg force-pushed the fix/print-hostbase-to-stderr branch from ffd4692 to 9d137a5 Compare March 28, 2025 07:33
@MalinAhlberg
Copy link
Copy Markdown
Member Author

I have applied Jocke's suggestion, rebased on main, and fixed the integration test.

Copy link
Copy Markdown
Contributor

@aaperis aaperis left a comment

Choose a reason for hiding this comment

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

Works great :-)

@MalinAhlberg MalinAhlberg added this pull request to the merge queue Mar 28, 2025
Merged via the queue into main with commit 17b5a9c Mar 28, 2025
6 checks passed
@MalinAhlberg MalinAhlberg deleted the fix/print-hostbase-to-stderr branch March 28, 2025 08:09
@MalinAhlberg MalinAhlberg self-assigned this Apr 2, 2025
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.

Information on remote server (host_base) should not be printed in stdout

4 participants