-
-
Notifications
You must be signed in to change notification settings - Fork 224
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
TASK: Introduce rebase failed exception and deprecate WorkspaceRebaseFailed
#4965
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.
I think one of those "merge rather now than later" kind 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.
Looks great and the change makes a lot of sense IMO.
Unfortunately, this still does not seem to allow discarding personal changes in order to solve a conflict:
4965-screenrecording.mov
is that expected?
yes, the whole publishing/discarding will be overhauled in neos/neos-ui#3744 |
WorkspaceRebaseFailed
Question: what happens with So can we get rid of this |
Yep, it's no longer needed afais |
Via neos#4965 the status REBASE_ERROR is obsolete. Instead of still using this status on replay we mimic the new logic: > In case of a [rebase error}, reopen the old content stream and remove the newly created Additionally, to not break `findContentStreams` when fetching all content streams we make sure that the streams with `REBASE_ERROR` are transformed to `NO_LONGER_IN_USE` so they can be cleaned up. Related neos#5101
…ng the exception message) While before neos#4965 a even `WorkspaceRebaseFailed` would contain bloats of data which grew a lot. A small message in the closed even is not a problem though.
…ng the exception message) While before neos#4965 a even `WorkspaceRebaseFailed` would contain bloats of data which grew a lot. A small message in the closed even is not a problem though.
This deprecates the WorkspaceRebaseFailed event with a likewise named exception.
Applications like the Neos UI need detailed information about what exactly failed. Storing this in an event incapacitates the event store quite fast since the amount of data gets quickly out of hand.
Also, we currently have no application for handling the stored data asynchronously. For endpoints like the Neos UI BackendServiceController we need the feedback directly anyway, so instead of waiting for the event to be serialized and deserialized again, they can deal with an exception a lot better.
All necessary information is stored in said exception and thrown with it, readily available for the initiating user to deal with it
Upgrade instructions
nothing yet
Review instructions
I'm not 100% sure about the workspace / content stream state handling. The conflict state is probably no longer necessary
Related: #4545
Checklist
FEATURE|TASK|BUGFIX
!!!
and have upgrade-instructions