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.

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

Driver & Autopilot: Remove deny_unknown_fields #3137

Merged
merged 3 commits into from
Nov 28, 2024

Conversation

m-lord-renkse
Copy link
Contributor

Description

Remove deny_unknown_fields from the Deserialized objects in order to avoid crashing when new fields are added.

@m-lord-renkse m-lord-renkse marked this pull request as ready for review November 27, 2024 15:48
@m-lord-renkse m-lord-renkse requested a review from a team as a code owner November 27, 2024 15:48
Copy link
Contributor

@MartinquaXD MartinquaXD left a comment

Choose a reason for hiding this comment

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

Let's be on the safe side and also remove the flag from autopilot <> driver communication

@m-lord-renkse
Copy link
Contributor Author

Let's be on the safe side and also remove the flag from autopilot <> driver communication

Good point! on it! 👌

@m-lord-renkse m-lord-renkse changed the title Driver: Remove deny_unknown_fields Driver & Autopilot: Remove deny_unknown_fields Nov 28, 2024
@m-lord-renkse m-lord-renkse merged commit b5fa428 into main Nov 28, 2024
11 checks passed
@m-lord-renkse m-lord-renkse deleted the driver-remove-deny-unknown-fields branch November 28, 2024 15:03
@github-actions github-actions bot locked and limited conversation to collaborators Nov 28, 2024
@sunce86
Copy link
Contributor

sunce86 commented Dec 1, 2024

Description

Remove deny_unknown_fields from the Deserialized objects in order to avoid crashing when new fields are added.

There are other ways to avoid crashing when new field is added, for example adding deserialization logic that temporarily accepts multiple versions of the data received.

deny_unknown_fields forces us to keep the communication API clean and up to date, and although it's easier to just remove it, it has it's purpose.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants