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
Bug 1538986 - Remove bad enum values from Update Request #713
Conversation
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.
ACK
I think that this line also needs to get updated so that we return the 2xx when the new no parameters changes error occurs. |
@shawn-hurley I think this is no longer an issue with the most recent set of changes. |
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.
Just one small question otherwise LGTM
pkg/broker/broker.go
Outdated
Method: apb.JobMethodUpdate, | ||
}) | ||
|
||
return &UpdateResponse{"noop-update"}, nil |
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.
This might seem silly, but this would be constantly overwriting the same job I believe. I wonder if generating a UUID for the token even for the noop
might make sense?
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.
okay, ack
@jmontleon needs a PR against release-1.1 branch to be included in 3.9 builds. |
Describe what this PR does and why we need it:
Fix a bug
Changes proposed in this pull request
Does this PR depend on another PR (Use this to track when PRs should be merged)
Doesn't depend on anything else.