Skip to content

MLE-13687 Able to use onError in plan builder #1657

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

Merged
merged 1 commit into from
Apr 9, 2024
Merged

Conversation

rjrudin
Copy link
Contributor

@rjrudin rjrudin commented Apr 8, 2024

Regenerated the code so that onError appears on ModifyPlan instead of PlanBuilder.

anu3990
anu3990 previously approved these changes Apr 8, 2024
BillFarber
BillFarber previously approved these changes Apr 9, 2024
@rjrudin rjrudin dismissed stale reviews from BillFarber and anu3990 via 66a4163 April 9, 2024 12:41
@rjrudin rjrudin force-pushed the feature/13687-backup branch from 98a1fbb to 66a4163 Compare April 9, 2024 12:41
@rjrudin rjrudin requested review from BillFarber and anu3990 April 9, 2024 12:42
@rjrudin
Copy link
Contributor Author

rjrudin commented Apr 9, 2024

No longer need to modify the generated code, as Jos added support for a string object as the first arg to onError.

Copy link
Contributor

@BillFarber BillFarber left a comment

Choose a reason for hiding this comment

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

[2024-04-09T12:47:38.907Z] ErrorListenerInputOutputEndpointTest > testInputOutputCallerWithStop() FAILED
[2024-04-09T12:47:38.907Z] org.opentest4j.AssertionFailedError at ErrorListenerInputOutputEndpointTest.java:219

[2024-04-09T12:48:05.458Z] OpticPatchTest > replaceInsertChild() FAILED
[2024-04-09T12:48:05.458Z] org.opentest4j.AssertionFailedError at OpticPatchTest.java:132

@rjrudin rjrudin force-pushed the feature/13687-backup branch from 66a4163 to 8373e44 Compare April 9, 2024 13:10
@rjrudin
Copy link
Contributor Author

rjrudin commented Apr 9, 2024

[2024-04-09T12:47:38.907Z] ErrorListenerInputOutputEndpointTest > testInputOutputCallerWithStop() FAILED [2024-04-09T12:47:38.907Z] org.opentest4j.AssertionFailedError at ErrorListenerInputOutputEndpointTest.java:219

[2024-04-09T12:48:05.458Z] OpticPatchTest > replaceInsertChild() FAILED [2024-04-09T12:48:05.458Z] org.opentest4j.AssertionFailedError at OpticPatchTest.java:132

The first is one of a handful of intermittent Bulk DS failures.

The second is interesting - it's not related to this PR. I believe a fix was made to 11-nightly because when I added this test yesterday, the expected behavior didn't seem correct. But now it does, and so I updated the test to match the behavior of the server. That behavior is - replaceInsertChild now replaces the existing element with text. It was not doing that yesterday, but it is today.

@rjrudin rjrudin requested a review from BillFarber April 9, 2024 13:11
BillFarber
BillFarber previously approved these changes Apr 9, 2024
Regenerated the code so that `onError` appears on `ModifyPlan` instead of `PlanBuilder`.
@rjrudin rjrudin force-pushed the feature/13687-backup branch from 8373e44 to 7b3e825 Compare April 9, 2024 13:41
@rjrudin rjrudin merged commit a864272 into develop Apr 9, 2024
@rjrudin rjrudin deleted the feature/13687-backup branch April 9, 2024 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants