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

charts/configmap: add S3GW_SERVICE_URL #113

Merged
merged 1 commit into from
Sep 20, 2023

Conversation

jecluis
Copy link
Contributor

@jecluis jecluis commented Jul 30, 2023

Adds a new configmap entry, pointing to the address of the s3gw service.

This will be consumed by the s3gw-ui backend.

We are leaving the original entry in there nonetheless so we don't break the UI if/when this gets merged, depending on whether changed to the UI get merged before or after. It should be removed in a coming release.

Signed-off-by: Joao Eduardo Luis <joao@suse.com>

@jecluis jecluis added the kind/enhancement Change that positively impacts existing code label Jul 30, 2023
@jecluis jecluis requested review from votdev and giubacc July 30, 2023 11:15
m-ildefons
m-ildefons previously approved these changes Jul 31, 2023
Copy link
Contributor

@m-ildefons m-ildefons left a comment

Choose a reason for hiding this comment

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

There are no technical reasons not to approve, but:

  • why the inconsistency ADDRESS/URL (also it's not an address, its a URL, right?)?
  • why not just use RGW_SERVICE_URL since it's already there?

@jecluis
Copy link
Contributor Author

jecluis commented Sep 13, 2023

There are no technical reasons not to approve, but:

* why the inconsistency `ADDRESS`/`URL` (also it's not an address, its a URL, right?)?

sure, could be _URL instead, that's easily fixable.

* why not just use `RGW_SERVICE_URL` since it's already there?

because this is s3gw, and I kind of want to minimize external exposure to rgw concepts.

charts/s3gw/templates/configmap.yaml Outdated Show resolved Hide resolved
Signed-off-by: Joao Eduardo Luis <joao@suse.com>
@jecluis
Copy link
Contributor Author

jecluis commented Sep 13, 2023

@m-ildefons @giubacc updated the PR to 1) make the variable _URL, and 2) to drop the unused RGW_ counterpart.

@jecluis jecluis changed the title charts/configmap: add S3GW_SERVICE_ADDRESS charts/configmap: add S3GW_SERVICE_URL Sep 13, 2023
@jecluis
Copy link
Contributor Author

jecluis commented Sep 13, 2023

to be merged after s3gw-tech/s3gw-ui#242

@jecluis jecluis merged commit b3a834d into s3gw-tech:main Sep 20, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Change that positively impacts existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants