Skip to content

List private definitions endpoint #11339

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

Merged
merged 4 commits into from
Mar 24, 2022
Merged

Conversation

git-phu
Copy link
Contributor

@git-phu git-phu commented Mar 23, 2022

What

Part of #9652
Tech Spec

Implements routes to list all private, non-custom definitions, and for each indicate whether the given workspace has a grant for using the definition. Used by admins to view and modify a given workspace's grants.
Depends on #11305 and #11336

Recommended reading order

Easiest to view by commit

User Impact

Doesn't change the behavior of existing routes

@github-actions github-actions bot added area/platform issues related to the platform area/server labels Mar 23, 2022
description: true if this connector definition is available to all workspaces
type: boolean
default: false
custom:
Copy link
Contributor

@michel-tricot michel-tricot Mar 23, 2022

Choose a reason for hiding this comment

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

What is the definition of custom vs public? I would be careful before adding that kind of specific flags on the data model. They risk proliferating.
Are they mutually exclusive? then it should be a enum and if we foresee that it is only these two states then just one boolean
Can it be true on both? false on both? what is the expected behavior in these cases?

Copy link
Contributor Author

@git-phu git-phu Mar 23, 2022

Choose a reason for hiding this comment

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

Great points. Let me copy this over to #11305 (which has these same migration and model changes broken out into its own PR) and continue the discussion there.
Worth discussing since there was a thread about this in the design doc as well

Discussion continued in #11305 (comment)

@git-phu git-phu force-pushed the peter/list-private-definitions branch from a11e3ba to bab7631 Compare March 23, 2022 23:28
@git-phu git-phu marked this pull request as ready for review March 23, 2022 23:28
@git-phu git-phu requested a review from pmossman March 23, 2022 23:28
@git-phu git-phu temporarily deployed to more-secrets March 24, 2022 00:56 Inactive
@git-phu git-phu temporarily deployed to more-secrets March 24, 2022 00:56 Inactive
@git-phu git-phu temporarily deployed to more-secrets March 24, 2022 02:25 Inactive
@git-phu git-phu temporarily deployed to more-secrets March 24, 2022 02:26 Inactive
@git-phu git-phu temporarily deployed to more-secrets March 24, 2022 02:59 Inactive
@git-phu git-phu temporarily deployed to more-secrets March 24, 2022 02:59 Inactive
Copy link
Contributor

@pmossman pmossman left a comment

Choose a reason for hiding this comment

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

Looks great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants