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

add formatting and JSON conversion utilities to ffi #1087

Merged
merged 5 commits into from
Jul 29, 2024
Merged

Conversation

khieta
Copy link
Contributor

@khieta khieta commented Jul 23, 2024

Description of changes

  • Adds JSON APIs to apply the policy formatter & convert between the Cedar/JSON formats for policies/schemas
  • Removes the corresponding definitions from cedar-wasm/
  • Tweaks an error message in the formatter
  • Fixes a typo in the wasm build script introduced by Also use macOS to test wasm build in CI #1078. Apparently & is a special character for sed in some positions.

PR to fix downstream breaks in wasm example: cedar-policy/cedar-examples#180

Issue #, if available

Resolves #854

Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

  • A change (breaking or otherwise) that only impacts unreleased or experimental code.

I confirm that this PR (choose one, and delete the other options):

  • Updates the "Unreleased" section of the CHANGELOG with a description of my change (required for major/minor version bumps).

I confirm that cedar-spec (choose one, and delete the other options):

  • Does not require updates because my change does not impact the Cedar formal model or DRT infrastructure.

@khieta khieta mentioned this pull request Jul 23, 2024
2 tasks
@khieta khieta marked this pull request as ready for review July 23, 2024 14:54
cedar-policy/Cargo.toml Show resolved Hide resolved
cedar-policy/src/ffi/convert.rs Outdated Show resolved Hide resolved
cedar-policy/src/ffi/format.rs Outdated Show resolved Hide resolved
cedar-policy/src/ffi/format.rs Show resolved Hide resolved
Comment on lines +32 to +33
/// Note that the [`Template`] type can represent both static policies and
/// policy templates.
Copy link
Contributor

Choose a reason for hiding this comment

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

Good note, I had forgotten this was the case in the FFI. It's not true of cedar_policy::Template.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was wondering as I wrote this if we should update the Policy type in the FFI to support both static policies and templates. Opinions? Should we even have a Template type at all?

Copy link
Contributor

Choose a reason for hiding this comment

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

Having Policy in the FFI support both static policies and templates might be additionally confusing because Policy in cedar_policy represents both static policies and template-linked policies, but not templates

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yuck. I'll leave it as-is then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made an issue #1095

khieta and others added 2 commits July 23, 2024 11:51
Co-authored-by: Craig Disselkoen <cdiss@amazon.com>
@khieta khieta merged commit 9ce9d7c into main Jul 29, 2024
19 of 20 checks passed
@khieta khieta deleted the khieta/ffi-utils branch July 29, 2024 15:07
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 parsing APIs to FFI
3 participants