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

Lean: Add parsing for duration in the datetime extension #471

Merged
merged 8 commits into from
Nov 25, 2024

Conversation

adpaco-aws
Copy link
Contributor

Issue #, if available: N/A

Description of changes

This PR adds parsing for duration strings in the datetime extension which is being introduced to the Lean models.

We define a parse function that attempts to parse a duration string from the back. This helps us avoid an issue with respect to our previous forward version where this greedy approach leads to failures due to the m unit being a substring of the ms unit. The changes also include unit tests for parsing duration strings.

Callouts

  • This is my first contribution in Lean so I'm open to all kinds of feedback.
  • It's likely that I'll later add a parse function for datetime values, so I'm not sure what's the best way to name the parse in these changes. Do we want another namespace for the duration definitions?

@adpaco-aws adpaco-aws requested a review from emina November 8, 2024 21:44
Copy link
Contributor

@emina emina left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Left some comments.

cedar-lean/Cedar/Spec/Ext/Datetime.lean Outdated Show resolved Hide resolved
cedar-lean/Cedar/Spec/Ext/Datetime.lean Outdated Show resolved Hide resolved
cedar-lean/Cedar/Spec/Ext/Datetime.lean Outdated Show resolved Hide resolved
cedar-lean/Cedar/Spec/Ext/Datetime.lean Outdated Show resolved Hide resolved
cedar-lean/UnitTest/Datetime.lean Show resolved Hide resolved
cedar-lean/Cedar/Spec/Ext/Datetime.lean Outdated Show resolved Hide resolved
cedar-lean/Cedar/Spec/Ext/Datetime.lean Show resolved Hide resolved
cedar-lean/Cedar/Spec/Ext/Datetime.lean Outdated Show resolved Hide resolved
cedar-lean/Cedar/Spec/Ext/Datetime.lean Outdated Show resolved Hide resolved
cedar-lean/Cedar/Spec/Ext/Datetime.lean Outdated Show resolved Hide resolved
Signed-off-by: Adrian Palacios <[email protected]>
@adpaco-aws adpaco-aws merged commit 99a9c77 into cedar-policy:main Nov 25, 2024
6 checks passed
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