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.

Sign up for GitHub

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

Jump to bottom

feat: uses moby to generate human friendly name for store #121

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: uses moby to generate human friendly name for store #121

wants to merge 1 commit into from

Conversation

gabrielbussolo
Copy link
Contributor

@gabrielbussolo gabrielbussolo commented Aug 5, 2023 •
edited

Description

This PR uses the moby (docker reference) names generator to give name to stores.

Previously, the --name flag was enforced by a check on the createStoreWithModel function, ensuring that the storeName would never be empty and preventing the generation of a name. As a result, attempting to create a store without specifying a name resulted in an error:

fga store create
Error: required flag(s) "name" not set

To address this issue, the verification for the --name flag was removed.

It aims to close #115 issue

A new dependency has been introduced to the project. If adding a new dependency is undesirable, it is possible to incorporate the relevant code directly into this codebase since it is a simple piece of code. :)

References

#115

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

@gabrielbussolo gabrielbussolo requested a review from a team as a code owner August 5, 2023 22:03
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 5, 2023 •
edited

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: gabrielbussolo / name: Gabriel Bussolo (2f30a96)

@gabrielbussolo gabrielbussolo mentioned this pull request Aug 5, 2023
Closed
@aaguiarz
Copy link
Member

aaguiarz commented Aug 8, 2023

Hey @gabrielbussolo!

I like this idea, but I'm not sure how useful it would be.

We tweaked the fga store create --model model.fga|.json command to use the file name as the default store name:

$ fga store create --model Model.fga

{
  "store": {
    "created_at":"2023-08-07T18:13:20.996575Z",
    "id":"01H78K87F4526MCW0173B59T3V",
    "name":"Model",
    "updated_at":"2023-08-07T18:13:20.996575Z"
  },
  "model": {
    "authorization_model_id":"01H78K87F9Y39TPX07D7AFM4Z6"
  }
}

What's the use case that you are thinking of?

@gabrielbussolo
Copy link
Contributor Author

Hey @gabrielbussolo!

I like this idea, but I'm not sure how useful it would be.

We tweaked the fga store create --model model.fga|.json command to use the file name as the default store name:

$ fga store create --model Model.fga

{
  "store": {
    "created_at":"2023-08-07T18:13:20.996575Z",
    "id":"01H78K87F4526MCW0173B59T3V",
    "name":"Model",
    "updated_at":"2023-08-07T18:13:20.996575Z"
  },
  "model": {
    "authorization_model_id":"01H78K87F9Y39TPX07D7AFM4Z6"
  }
}

What's the use case that you are thinking of?

hey @aaguiarz !

well tbh im not so sure of how much this feature can help (im still learning the project), i noticed it on this discussion #115 (comment) the suggestion, and the issue remained open, so my main goal was to close it.

but my take of it is with that we can just cli store create to have:

{
  "store": {
    "created_at":"2023-08-10T12:49:56.691648Z",
    "id":"01H7FQY6YEPMRAGGEZTW2CCEQX",
    "name":"sleepy_jang",
    "updated_at":"2023-08-10T12:49:56.691648Z"
  }
}

model not required before hand and neither name.

@gabrielbussolo
feat: uses moby to generate human friendly name for store
2f30a96 
Signed-off-by: Gabriel Bussolo <contact@gabrielbussolo.dev>
@gabrielbussolo gabrielbussolo force-pushed the 115-human-friendly-name-for-Store-creation-via-cli branch from 30eafc8 to 2f30a96 Compare September 25, 2023 06:30
@gabrielbussolo
Copy link
Contributor Author

This still relevant @rhamzeh @aaguiarz ? If it's not anymore, we can close this PR and the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Human friendly name for Store creation via cli
2 participants
@gabrielbussolo @aaguiarz