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

chore: use --update-if-exists when creating broker #1058

Merged
merged 2 commits into from
Feb 16, 2024
Merged

Conversation

blgm
Copy link
Member

@blgm blgm commented Feb 15, 2024

Previously we would always try and create the broker, and if that failed (for any reason) then attempt to update it. This resulted in output that sometimes confused users.

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/187056373

The labels on this github issue will be updated when the story is started.

Copy link
Contributor

@fnaranjo-vmw fnaranjo-vmw left a comment

Choose a reason for hiding this comment

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

This looks great.
I also saw you contributed the implementation of this flag:
cloudfoundry/cli#2402

Do we need to keep anything in mind?
Do we need to ensure anything in our tiles?
What are the implications of --space-scoped being ignored (as you describe in the PR for implementing this functionality)?:
https://github.com/cloudfoundry/cli/pull/2402/files#diff-d7f338e1080712cfb134a0de446ec4cfc0878968f3dd517dbb841cdc9cb03ef8R18

Is it relevant that our script was using --space-scoped? And the fact that we haven't removed it but now will get ignored?

After reading the implementation PR and CloudFoundry docs I believe I understand that:
If the service-broker already exists but it was not created with --space-scoped flag the update command will not:

  • Try to convert the service-broker into a --space-scoped one
  • Fail or warn the user about this inconsistency

Is it possible to query whether a service-broker is space scoped or not?
If the service-broker exists already. Wouldn't be more sensible to either:

  • check that any passed flag if that flag actually describes the existing service-broker otherwise fail
  • continue using the pipe approach which at least offers a clear separation of flags

I acknowledge this inconsistency was not created by your PR.
I see that update-service-broker command doesn't accept the --service-scoped command.
I guess that means that once a service-broker gets created it is impossible to modify it with a single command to become a --space-scoped one. I guess we should have to live with it, although it seems confusing to me.

@blgm blgm merged commit 5e0e7be into main Feb 16, 2024
4 checks passed
@blgm blgm deleted the updateifexists branch February 16, 2024 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants