-
Notifications
You must be signed in to change notification settings - Fork 28
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
Adding quip as a cedar use case #201
base: main
Are you sure you want to change the base?
Conversation
Thank you for your contribution. I was wondering if this PR should be based on the |
Good point, I've updated the base branch. Thank you! |
This is great! I was a little surprised only to see |
Thanks! I modeled the entities off of the official quip documentation: https://quip.com/dev/automation/documentation/current#tag/Threads. Note, there are extra attributes in the documentation that I did not include for simplicity. |
id: String, | ||
name: String, | ||
disabled: Bool, | ||
shared_folder_ids?: Set<Folder>, // folders that are explicitly shared with the user |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious: why is this field (and other Set-typed fields below) optional? If there are no folders shared with a user, this can be modeled as a required Set attribute, where the Set is empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick conjectured answer (but curious about the correct answer): Maybe since it is possible to test that shared_folder_ids
is present, but not (at the moment) possible to set whether a set is empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good point; that makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats a good point! I cant remember if I had tried the empty set but had remembered seeing the has
keyword for policies with optional fields so went that route. Im still fairly novice with cedar syntax, but like Mike said I'm unsure if theres a way to test whether a set is empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A quick update. We've recently added the .isEmpty
operator on sets, so if that was the main reason for avoiding empty sets, we now have a nice solution for it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(The new operator is coming in version 4.3.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed this PR was still open and wanted to let you know that version 4.3, which includes the .isEmpty()
function, is available now. The function is also documented here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Apologies, did not follow up on this. Will update this PR using isEmpty().
Issue #, if available: N/A
Description of changes: Adding Quip under use cases. Included schema, policies, entity data, and sample authorization calls in a run.sh script.