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

Object-Entity Mapping (OEM) #72

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Object-Entity Mapping (OEM) #72

wants to merge 2 commits into from

Conversation

shaobo-he-aws
Copy link
Contributor

@shaobo-he-aws shaobo-he-aws commented Jun 29, 2024

Signed-off-by: Shaobo He <shaobohe@amazon.com>
text/0072-oem.md Outdated

Given limited knowledge about Rust macros the proposer has, the design details are subject to significant changes.

The macros over a Rust type eventually expand to implementations of `Into<Entities>`. In the meantime, they also give rise to implementations of `Into<Value>` for this type. By default, the implementation of `Into<Value` for a type produces an entity `uid` unless the type is annotated with the `#[cedar(transparent)]` clause, where it produces a record.
Copy link
Contributor

@john-h-kastner-aws john-h-kastner-aws Jul 1, 2024

Choose a reason for hiding this comment

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

produces an entity uid unless the type is annotated with the #[cedar(transparent)] clause

I would rather see this made more explicit. E.g., #[cedar(record)] or #[cedar(entity)], perhaps with the entity case being assumed by default.

This could also support a nice syntax like #[cedar(long)] struct MyLong(i32) for wrapper types that might exist.

@john-h-kastner-aws john-h-kastner-aws added the pending This RFC is pending; for definitions see the README label Jul 1, 2024
Signed-off-by: Shaobo He <shaobohe@amazon.com>
@khieta
Copy link
Contributor

khieta commented Jul 8, 2024

Thanks for the writeup @shaobo-he-aws! I think this looks like a useful addition to Cedar, but I disagree that it should be an RFC. It's a change to the Rust API rather than the Cedar language, so I think it should remain as an issue in the cedar repository. Maybe you could move the details here into cedar#986?

How do others feel about an RFC vs. issue here?

@shaobo-he-aws
Copy link
Contributor Author

Thanks for the writeup @shaobo-he-aws! I think this looks like a useful addition to Cedar, but I disagree that it should be an RFC. It's a change to the Rust API rather than the Cedar language, so I think it should remain as an issue in the cedar repository. Maybe you could move the details here into cedar#986?

How do others feel about an RFC vs. issue here?

I'm afraid that moving it to an issue reduces discussion and eventually makes it buried in the backlog list.

@cdisselkoen
Copy link
Contributor

It's a change to the Rust API rather than the Cedar language, so I think it should remain as an issue

We've previously done RFCs for other major changes that aren't technically changes to the Cedar language itself, for example, RFC 32 and arguably RFC 8. Similarly, I think there is consensus that partial evaluation itself would have been an RFC except that its development predates the RFC process. I think that this is a major enough change that an RFC is warranted, and particularly since @shaobo-he-aws has already gone to the trouble of writing one up, I think it makes sense to treat it as an RFC. If we didn't already have an RFC writeup and the primary author was pushing hard to keep this an issue instead of an RFC, I could see us maybe making a different decision on RFC vs issue, but since in this case the primary author is welcoming the RFC process, I see no reason to deny that.

I'm afraid that moving it to an issue reduces discussion and eventually makes it buried in the backlog list.

Even getting an RFC accepted (into Active state) does not promise any particular implementation priority, and the resulting implementation issue could easily get no discussion and become buried in the backlog, like, for example, RFC 53, where it took almost 3 months for the implementation issue to even be made. The RFC process isn't a magic way to get implementation prioritized, vs any other issues in the backlog. See the RFC life-cycle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending This RFC is pending; for definitions see the README
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants