Skip to content

feat(project): add pydantic validator for common_id field#5974

Open
soumyaDghosh wants to merge 1 commit intocanonical:mainfrom
soumyaDghosh:pydantic-common-id
Open

feat(project): add pydantic validator for common_id field#5974
soumyaDghosh wants to merge 1 commit intocanonical:mainfrom
soumyaDghosh:pydantic-common-id

Conversation

@soumyaDghosh
Copy link
Contributor

fixes #5624


  • I've followed the contribution guidelines.
  • I've signed the CLA.
  • I've successfully run make lint && make test.
  • I've added or updated any relevant documentation.
  • I've updated the relevant release notes.

Signed-off-by: Soumyadeep Ghosh <soumyadeepghosh2004@zohomail.in>
Copy link
Collaborator

@mr-cal mr-cal left a comment

Choose a reason for hiding this comment

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

This will be a great improvement, thanks @soumyaDghosh!

Currently, neither snap pack nor review-tools.snap-review catch invalid common-ids, right? (I'm testing locally and can't get either to raise an error). The reason I'm asking is that I'm considering putting this in Snapcraft 9 instead of 8.15.


See :ref:`how-to-configure-package-information` for details.
The common-id must be in reverse DNS format and consist only of alphanumeric characters and the
following special characters: ``., _, -``.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
following special characters: ``., _, -``.
following special characters: ``.``, ``_``, ``-``.


def test_app_common_id(self, app_yaml_data):
data = app_yaml_data(common_id="test-common-id")
def test_app_valid_common_id(self, app_yaml_data):
Copy link
Collaborator

Choose a reason for hiding this comment

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

To exercise the regex, can you do a pytest.mark.parameterize for this test and the next test to test a few valid and invalid common ids?

And can you add id's for the invalid ones to explain them? (for example, @pytest.param("te$t", id="invalid-character"))

)
return extensions

@pydantic.field_validator("common_id")
Copy link
Collaborator

Choose a reason for hiding this comment

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

@lengau, please correct me here, but I believe the preferred way to add new regex validation is like this:

https://github.com/canonical/craft-application/blob/cfd4558920c6ac936d642059b26c9693b0b45828/craft_application/models/constraints.py#L162-L197

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.

Add a pydantic validator for common-id

2 participants