feat : Add error attribute to Collection Indexes and Attributes#4575
Conversation
eldadfux
left a comment
eldadfux
left a comment
There was a problem hiding this comment.
Great work, left a few comments and points for discussion. Some extra adjustments in utopia/database might be required.
| 'indexes', | ||
| $index->getId(), | ||
| $index | ||
| ->setAttribute('status', 'stuck') |
There was a problem hiding this comment.
| ->setAttribute('status', 'stuck') | |
| ->setAttribute('status', 'stuck') |
What status types do we support? can we change this to error? cc @christyjacob4
There was a problem hiding this comment.
We currently support
- available
- failed
- stuck
- deleting
- processing
There was a problem hiding this comment.
failed might make the most sense here. Do we want to deprecate the stuck status ?
| 'attributes', | ||
| $attribute->getId(), | ||
| $attribute | ||
| ->setAttribute('status', 'stuck') |
There was a problem hiding this comment.
We should deprecate stuck and just use failed instead
There was a problem hiding this comment.
considering this comment here, i think changing it to failed can be misleading.
failed indicates the attribute was never created in the db, whereas stuck
indicates the attribute is available in the db but the deletion operation failed. Since the exception thrown here would only be of a case where the attribute is available in the db, I think stuck makes sense by the definition. If we want to deprecate stuck and go with failed i think we should redefine failed and what it means before doing so.
# Conflicts: # CHANGES.md # app/workers/databases.php # composer.json # composer.lock
4 participants
What does this PR do?
Add error attribute to Collection Indexes and Attributes
Test Plan
Related PRs and Issues
NIL
Have you added your change to the Changelog?
yes
Have you read the Contributing Guidelines on issues?
yes