Skip to content
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

fix(stdlib): ignore unknown messages rather than erroring #5009

Merged
merged 1 commit into from
Jul 20, 2022
Merged

fix(stdlib): ignore unknown messages rather than erroring #5009

merged 1 commit into from
Jul 20, 2022

Conversation

jsternberg
Copy link
Contributor

@jsternberg jsternberg commented Jul 20, 2022 •
edited
Loading

The process message method is intended to ignore messages it does not
understand rather than actively return an error. This removes the error
from join for an invalid message.

Done checklist

  • docs/SPEC.md updated
  • Test cases written

@jsternberg jsternberg requested a review from a team as a code owner July 20, 2022 16:13
@jsternberg jsternberg requested review from Rahul-Vasan and onelson and removed request for a team July 20, 2022 16:13
onelson
onelson approved these changes Jul 20, 2022
View reviewed changes
Copy link
Contributor

@onelson onelson left a comment

Choose a reason for hiding this comment

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

lgtm.

I wonder if there should be a comment added to the source to indicate the proper usage/intent. On its face, the change looks like swallowing an error and I'd naturally look for ways to avoid doing so unless there was something to indicate I should.

@jsternberg
fix(stdlib): ignore unknown messages rather than erroring
8877a2e 
The process message method is intended to ignore messages it does not
understand rather than actively return an error. This removes the error
from join for an invalid message.
@jsternberg jsternberg force-pushed the fix/join-invalid-message branch from 26929e2 to 8877a2e Compare July 20, 2022 16:25
@jsternberg
Copy link
Contributor Author

I updated the docs for execute.Transport to call out this usage explicitly.

@jsternberg jsternberg merged commit d061ee4 into master Jul 20, 2022
@jsternberg jsternberg deleted the fix/join-invalid-message branch July 20, 2022 16:57
@scbrickley scbrickley mentioned this pull request Jul 27, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@onelson onelson onelson approved these changes

@Rahul-Vasan Rahul-Vasan Awaiting requested review from Rahul-Vasan Rahul-Vasan was automatically assigned from influxdata/flux-team

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jsternberg @onelson