Skip to content

PHPC-1419: Expose error labels from getMore responses #1018

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 2, 2019
Merged

PHPC-1419: Expose error labels from getMore responses #1018

merged 1 commit into from
Sep 2, 2019

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Aug 29, 2019

@alcaeus alcaeus self-assigned this Aug 29, 2019
@alcaeus alcaeus requested a review from jmikola August 29, 2019 10:54
@alcaeus alcaeus force-pushed the phpc-1419 branch 2 times, most recently from a5e4e89 to 227ef6e Compare August 29, 2019 11:35
jmikola
jmikola approved these changes Aug 30, 2019
View reviewed changes
Copy link
Member

@jmikola jmikola left a comment

Choose a reason for hiding this comment

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

Suggestion for renaming test, but LGTM otherwise.

tests/cursor/bug1419-001.phpt Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I'd replace handled with "exposed", since it's both clearer and matches the ticket title

tests/cursor/bug1419-001.phpt Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Is the error code sufficient for the fail point to also apply the label? I see the label defined as a category in error_codes.yml but didn't realize that would work with failCommand. I suppose it makes sense, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, the label is automatically added based on the error code. IIRC, you can't define error labels through the failCommand fail point.

tests/cursor/bug1419-001.phpt Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I assume this is necessary because the error label used below was introduced in 4.1.11 with SERVER-40446?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is correct. I've updated the check to skip 4.1.11 and earlier.

@jmikola jmikola mentioned this pull request Aug 30, 2019
Merged
@alcaeus alcaeus merged commit 183a207 into mongodb:master Sep 2, 2019
@alcaeus alcaeus deleted the phpc-1419 branch September 2, 2019 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jmikola jmikola jmikola approved these changes

Assignees

@alcaeus alcaeus

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@alcaeus @jmikola