Skip to content

V3 handle last operation on unbind route service 174748133#1913

Merged
FelisiaM merged 7 commits intomasterfrom
v3_handle_last_operation_on_unbind_route_service_174748133
Oct 20, 2020
Merged

V3 handle last operation on unbind route service 174748133#1913
FelisiaM merged 7 commits intomasterfrom
v3_handle_last_operation_on_unbind_route_service_174748133

Conversation

@FelisiaM
Copy link
Copy Markdown
Member

@cf-gitbot
Copy link
Copy Markdown

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

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

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

Comment thread app/jobs/v3/delete_route_binding_job.rb Outdated
Comment thread app/jobs/v3/delete_route_binding_job.rb Outdated
self.polling_interval_seconds = polling_status[:retry_after]
end
rescue => e
if route_binding.reload.last_operation.state != 'failed'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You could technically skip this and always save "state failed" 🤔

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the intention here was to not change the description - as set by the previous steps. Happy to change it if you think it makes sense.

E.g poll can through LastOperationFailed and also sets the state to failed with the right description.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fair enough. I was assuming that whatever is raising, is doing it with the correct error message. But maybe not?

Comment thread app/jobs/v3/delete_route_binding_job.rb Outdated
Comment on lines +8 to +14
DeleteStatus = Struct.new(:finished, :operation).freeze
DeleteStarted = ->(operation) { DeleteStatus.new(false, operation) }
DeleteComplete = DeleteStatus.new(true, nil).freeze

PollingStatus = Struct.new(:finished, :retry_after).freeze
PollingFinished = PollingStatus.new(true, nil).freeze
ContinuePolling = ->(retry_after) { PollingStatus.new(false, retry_after) }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Starting to think we may want to extract this into a common place (create has similar structs)... But its fine as is for now

Comment thread app/actions/service_route_binding_delete.rb Outdated
Copy link
Copy Markdown

@kirederik kirederik left a comment

Choose a reason for hiding this comment

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

See comments :D

@FelisiaM FelisiaM requested a review from kirederik October 19, 2020 15:53
@FelisiaM FelisiaM merged commit ab01bc9 into master Oct 20, 2020
@FelisiaM FelisiaM deleted the v3_handle_last_operation_on_unbind_route_service_174748133 branch October 20, 2020 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants