Skip to content

Remove pin in dependencies for jsonschema (SC-508)#1078

Merged
blackboxsw merged 1 commit into
canonical:mainfrom
TheRealFalcon:fix-jsonschema-tests
Oct 23, 2021
Merged

Remove pin in dependencies for jsonschema (SC-508)#1078
blackboxsw merged 1 commit into
canonical:mainfrom
TheRealFalcon:fix-jsonschema-tests

Conversation

@TheRealFalcon
Copy link
Copy Markdown
Contributor

Remove pin in dependencies for jsonschema

In jsonschema 4, hostname validation was changed to have an optional
dependency on the fqdn package. Since we don't have this dependency
in cloud-init, attempting this validation will no longer fail for
a string that isn't a valid hostname.

Additional Context

This is to fix two failing unit tests.

Are we ok with this validation being disabled? It is currently in use in cc_ntp.py. Nothing is broken by it not being there, it will just validate it as a string rather than as a hostname.

@TheRealFalcon TheRealFalcon changed the title Remove pin in dependencies for jsonschema Remove pin in dependencies for jsonschema (SC-508) Oct 22, 2021
Copy link
Copy Markdown
Member

@holmanb holmanb left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Copy link
Copy Markdown
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

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

+1. Not really gaining a whole lot by the hostname validation.

@blackboxsw blackboxsw merged commit 2db7133 into canonical:main Oct 23, 2021
holmanb pushed a commit to holmanb/cloud-init that referenced this pull request Nov 1, 2021
In jsonschema 4, hostname validation was changed to have an optional
dependency on the fqdn package. Since we don't have this dependency
in cloud-init, attempting this validation will no longer fail for
a string that isn't a valid hostname.
@TheRealFalcon TheRealFalcon deleted the fix-jsonschema-tests branch March 21, 2025 18:52
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