-
Notifications
You must be signed in to change notification settings - Fork 7
appnote: Fix error in deleting object instances #152
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
Conversation
danjhd
left a comment
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.
LGTM
danjhd
left a comment
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.
LGTM
j616
left a comment
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.
One Q. Otherwise LGTM
| If a client needs to modify a Flow Segment, e.g. to correct metadata such as the `key_frame_count` or add additional URLs to `get_urls`, then the client SHOULD first delete the existing Segment and then write a new one. | ||
| Clients MAY modify Flow Segments, but this should only be done in exceptional circumstances to correct metadata such as `key_frame_count`, as such operations will likely break the idempotency of Segments. | ||
| If a client needs to modify a Flow Segment, then the client SHOULD first delete the existing Segment and then write a new one. |
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.
Should we also add a note, like those we have elsewhere, warning that setting key_frame_count to a value that conflicts with the Object SHOULD be rejected if that object is in use elsewhere? i.e. don't bring references into conflict with each other.
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.
Should clients ignore it if the object is re-used? That would seem like the easiest behaviour to implement I suppose
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 was thinking something like the last para of the main description on https://bbc.github.io/tams/main/index.html#/operations/PUT_flows-flowId . i.e. allow modification if it won't bring anything into conflict. Reject if it will.
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.
After discussion, tweaked in 0338ef6 to make expectation clearer
Reworks description that modifying `key_frame_count` can be achieved by deleting the segment and re-creating if need be, but that the list of instances can be freely modified.
Makes clear that attempts to modify `key_frame_count` when an object has been re-used elsewhere is not acceptable. Also, trivial wording tweak to make documentation clearer.
0338ef6 to
742229e
Compare
Details
get_urlsis handled byPOST /objects/{objectId}/instances- modifyingkey_frame_countrequires deleting and recreating the segment, and should only be performed exceptionally.Jira Issue (if relevant)
Jira URL: N/A (bug report on Slack)
Related PRs
Fixes errors introduced in #144 (but as-yet unreleased)
Submitter PR Checks
(tick as appropriate)
Reviewer PR Checks
(tick as appropriate)
Info on PRs
The checks above are guidelines. They don't all have to be ticked, but they should all have been considered.