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

fix(project): Use an annotated type for duration strings #5213

Open
wants to merge 4 commits into
base: hotfix/8.6
Choose a base branch
from

Conversation

lengau
Copy link
Contributor

@lengau lengau commented Jan 21, 2025

Inspired by: #5210

This makes an annotated type for duration strings and uses a stricter regex.

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run tox run -m lint?
  • Have you successfully run tox run -e test-py310? (supported versions: py39, py310, py311, py312)

@@ -40,6 +40,8 @@

# required project data for core24 snaps
CORE24_DATA = {"base": "core24", "grade": "devel"}
VALID_DURATIONS = ["10ns", "10us", "10ms", "10s", "10m", "10m4s3us"]
INVALID_DURATIONS = ["10", "10 s", "10 seconds", "1:00", "invalid"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Surprisingly, just an integer isn't a valid duration. The current snapcraft will error out during the packing step:

$ snapcraft_edge pack
Generated snap metadata
Cannot pack snap: error: cannot parse snap.yaml: time: missing unit in duration "10"
Detailed information: Command '['snap', 'pack', '--check-skeleton', PosixPath('/root/prime')]' returned non-zero exit status 1.
Failed to execute pack in instance.
Recommended resolution: Run the same command again with --debug to shell into the environment if you wish to introspect this failure.
Full execution log: '/home/lengau/.local/state/snapcraft/log/snapcraft-20250121-181707.415584.log'

@lengau lengau changed the base branch from main to hotfix/8.5 January 21, 2025 23:31
@@ -40,6 +40,8 @@

# required project data for core24 snaps
CORE24_DATA = {"base": "core24", "grade": "devel"}
VALID_DURATIONS = ["10ns", "10us", "10ms", "10s", "10m", "10m4s3us"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a separate data entry for a valid duration here, as "10m4s3us" is a valid duration string. Snapd uses time.ParseDuration to parse this, and the resulting service file contains a value something like TimeoutStartSec=604.000003.

@lengau lengau mentioned this pull request Jan 21, 2025
4 tasks
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.

Thank you!

Do you mind targeting hotfix/8.6? The hotfix/8.5 branch is closing in favor of 8.6 this week and I can get this into 8.6.0 or 8.6.1, depending on the timing. It may require a touch of rebasing as Sergio updated the project model.

Inspired by: #5210

This makes an annotated type for duration strings and uses a stricter regex.
@lengau lengau force-pushed the work/better-duration-regex branch from d017a82 to 12d16bf Compare January 22, 2025 23:27
@lengau lengau changed the base branch from hotfix/8.5 to hotfix/8.6 January 22, 2025 23:28
@lengau
Copy link
Contributor Author

lengau commented Jan 22, 2025

@mr-cal done! I'll take this out of draft once I ensure CI is still succeeding, and then since it was his fine work that gave me a conflict during rebase I'll ask @sergiusens to review.

@lengau lengau requested a review from sergiusens January 23, 2025 00:13
@lengau lengau marked this pull request as ready for review January 23, 2025 00:13
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.

2 participants