-
Notifications
You must be signed in to change notification settings - Fork 42
fix null value from get existing offering and service plan during binding failure #1716
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
fix null value from get existing offering and service plan during binding failure #1716
Conversation
b18b2f4 to
65a8b56
Compare
65a8b56 to
74d2380
Compare
| CloudControllerClient client = context.getControllerClient(); | ||
|
|
||
| String serviceInstanceName = context.getVariable(Variables.SERVICE_TO_UNBIND_BIND); | ||
| if (serviceInstanceName == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extract into a new method whole logic related with getting service instance name and avoid setting finalServiceInstanceName because it is little bit confusing to have 2 variables for the same thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed! Done!
|
|
||
| return serviceBindingJob -> context.getStepLogger() | ||
| .error(buildErrorMessage(app, serviceInstance, serviceInstanceName, serviceBindingJob)); | ||
| CloudServiceInstance serviceInstance = (serviceInstanceName != null) ? client.getServiceInstance(serviceInstanceName, false) : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
brackets can be omitted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed!
| if (serviceInstanceName == null) { | ||
| CloudServiceBinding serviceBinding = context.getVariable(Variables.SERVICE_BINDING_TO_DELETE); | ||
| if (serviceBinding != null) { | ||
| serviceInstanceName = client.getServiceInstanceName(serviceBinding.getServiceInstanceGuid()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't notice up until now that this actually perform another call to the cf api, can you search for another way to retrieve service instance name, from different variable or pass service instance name from different process diagram. We are trying to avoid unnecessary http calls to the cf api because this increase chances to hit rate limit at user level.
No description provided.