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

update to fix mtu int error #247

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

update to fix mtu int error #247

wants to merge 3 commits into from

Conversation

susanhooks
Copy link
Contributor

The fix added in is lines 93-98. The rest is formatting.

Discovered when building a fresh instance from develop, using SSoT 3.1.0 and dependencies, a Pydantic error was raised when running the sync network data job on the interface field MTU. Added in an override to force a string on this field.

Copy link
Contributor

@glennmatthews glennmatthews left a comment

Choose a reason for hiding this comment

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

Feels weird to me to coerce MTU to a string. Shouldn't we be doing the opposite - fixing whatever is expecting a string to properly use an int?

@Dav-C
Copy link
Contributor

Dav-C commented Oct 4, 2024

Feels weird to me to coerce MTU to a string. Shouldn't we be doing the opposite - fixing whatever is expecting a string to properly use an int?

This started when the models were first defined, it turned out to be easier to default to strings for all the data parsed from a device and handle any required conversions when the object was saved.

@susanhooks
Copy link
Contributor Author

Addresses Issue #248

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