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

Miscellaneous fixes #945

Merged
merged 3 commits into from
Dec 8, 2023
Merged

Miscellaneous fixes #945

merged 3 commits into from
Dec 8, 2023

Conversation

anoopcs9
Copy link
Collaborator

@anoopcs9 anoopcs9 commented Dec 8, 2023

  • Fix versions for recent API additions
  • Remove a TODO
  • Better clarity on S3 credentials inside script

Workflow to automatically create a GitHub issue with a template
listing required API updates is in place for a while. Let's remove
the corresponding TODO from the file.

Signed-off-by: Anoop C S <[email protected]>
It's unusual to put any kind of credentials out in open. The set
of credentials defined with `S3_ACCESS_KEY` and `S3_SECRET_KEY`
variables in the script are questionable in its nature. But these
are not real/valid credential values in any form for AWS rather
used for testing the S3 compatible API from Ceph RGW. Therefore
clarify the intention and replace with sample values from official
AWS documentation.

Signed-off-by: Anoop C S <[email protected]>
@anoopcs9 anoopcs9 added the no-API This PR does not include any changes to the public API of a go-ceph package label Dec 8, 2023
Copy link
Collaborator

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

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

lgtm

@mergify mergify bot merged commit 09d81f5 into ceph:master Dec 8, 2023
15 checks passed
@anoopcs9 anoopcs9 deleted the misc-fixes branch December 12, 2023 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-API This PR does not include any changes to the public API of a go-ceph package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants