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

feat(import): add progress bar for store import #348

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

Siddhant-K-code
Copy link
Contributor

Description

This pull request adds a progress bar to the store import feature. Previously, the import process did not provide any visual feedback to the user, making it difficult to determine the progress. With this change, a progress bar is displayed, indicating the number of tuples being imported. This improves the user experience and provides better visibility into the import process.

See video below in slow-mode (or test with super long file) :

Screen.Recording.2024-06-14.at.4.58.03.PM.mov

How to test

  • Open this PR in Gitpod (it would automatically run the required commands to install deps.) or in local.

  • Add a demo.fga.yaml file under cmd/fga

  • Under cmd/fga directory run the following command:

    go run main.go store import --file <your_fga_yml_file>.fga.yaml
Demo file
# /workspace/cli/cmd/fga/test.fga.yaml

name: Model Tests # optional

# model_file: ./model.fga # you can specify an external .fga file, or include it inline
model: |
  model
    schema 1.1

  type user

  type organization
     relations
       define member : [user]
       define admin : [user with non_expired_grant]

   condition non_expired_grant(current_time: timestamp, grant_time: timestamp, grant_duration: duration) {
     current_time < grant_time + grant_duration
  }

# tuple_file: ./tuples.yaml # you can specify an external file, or include it inline
tuples: 

   # Anne is a member of the Acme organization
  - user: user:anne
    relation: member
    object: organization:acme

  # Peter has the admin role from February 2nd 2024 0AM to 1AM
  - user: user:peter
    relation: admin
    object: organization:acme
    condition: 
      name: non_expired_grant
      context: 
        grant_time : "2024-02-01T00:00:00Z"
        grant_duration : 1h
  - user: user:peter
    relation: admin
    object: organization:acme
    condition: 
      name: non_expired_grant
      context: 
        grant_time : "2024-02-01T00:00:00Z"
        grant_duration : 1h
  - user: user:peter
    relation: admin
    object: organization:acme
    condition: 
      name: non_expired_grant
      context: 
        grant_time : "2024-02-01T00:00:00Z"
        grant_duration : 1h
  - user: user:peter
    relation: admin
    object: organization:acme
    condition: 
      name: non_expired_grant
      context: 
        grant_time : "2024-02-01T00:00:00Z"
        grant_duration : 1h
  - user: user:peter
    relation: admin
    object: organization:acme
    condition: 
      name: non_expired_grant
      context: 
        grant_time : "2024-02-01T00:00:00Z"
        grant_duration : 1h

References

fixes #320

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

@Siddhant-K-code Siddhant-K-code requested a review from a team as a code owner June 14, 2024 11:38
Copy link

Minder Vulnerability Report ✅

Minder analyzed this PR and found no vulnerable dependencies.

Vulnerability scan of 43b035e5:

  • 🐞 vulnerable packages: 0
  • 🛠 fixes available for: 0

cmd/store/import.go Outdated Show resolved Hide resolved
@Siddhant-K-code
Copy link
Contributor Author

Here is how it would look: (Added custom time sleep to record demo)

Screen.Recording.2024-06-14.at.6.04.59.PM.mov

@Siddhant-K-code
Copy link
Contributor Author

Finally, the lints are passing now (in my fork -- ref). Checks on this PR needs to be approved by collaborators.

:shipit:

Copy link
Member

@rhamzeh rhamzeh left a comment

Choose a reason for hiding this comment

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

Thanks @Siddhant-K-code ! This is awesome!

Because we don't do squash merging on the CLI, may I ask you to squash your commits into the first one?

@Siddhant-K-code
Copy link
Contributor Author

Hey @rhamzeh 👋🏼

I've squashed all of my commits to e3b63f5

Copy link
Member

@ewanharris ewanharris left a comment

Choose a reason for hiding this comment

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

Awesome PR @Siddhant-K-code! I've just some comments about an issue arise during importing a new store and flickering of the progress bar

cmd/store/import.go Show resolved Hide resolved
cmd/store/import.go Outdated Show resolved Hide resolved
cmd/store/import.go Show resolved Hide resolved
@Siddhant-K-code
Copy link
Contributor Author

Why lints tests are failing? it is also not showing any error 🤔

image

@ewanharris
Copy link
Member

ewanharris commented Jun 18, 2024

The linter seems to have some awkward output with the lints being flagged occurring before that output (lines 31-36), output is below, I think we might be able to have the action annotate with these but will need to check it out

cmd/store/import.go:48: line is 176 characters (lll)
func createStore(clientConfig *fga.ClientConfig, storeData *storetest.StoreData, format authorizationmodel.ModelFormat, fileName string) (*CreateStoreAndModelResponse, error) {
cmd/store/import.go:65: line is 203 characters (lll)
func updateStore(clientConfig *fga.ClientConfig, fgaClient client.SdkClient, storeData *storetest.StoreData, format authorizationmodel.ModelFormat, storeID string) (*CreateStoreAndModelResponse, error) {
cmd/store/import.go:98: line is 264 characters (lll)
func importStore(clientConfig *fga.ClientConfig, fgaClient client.SdkClient, storeData *storetest.StoreData, format authorizationmodel.ModelFormat, storeID string, maxTuplesPerWrite, maxParallelRequests int, fileName string) (*CreateStoreAndModelResponse, error) {

@ewanharris
Copy link
Member

I pushed the lint fixes and squashed the commits down

ewanharris
ewanharris previously approved these changes Jun 18, 2024
Copy link
Member

@ewanharris ewanharris left a comment

Choose a reason for hiding this comment

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

Thanks for another awesome contribution @Siddhant-K-code! ❤️

We're going to file some follow on issues here such as mirroring this into fga tuple write but we're going to land and release this before that.

@ewanharris ewanharris added this pull request to the merge queue Jun 18, 2024
Merged via the queue into openfga:main with commit 4ffa7ba Jun 18, 2024
13 checks passed
@Siddhant-K-code Siddhant-K-code deleted the fix/320 branch June 18, 2024 17:36
This was referenced Jun 18, 2024
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 a progress bar to fga store import
3 participants