Skip to content
This repository has been archived by the owner on Jun 1, 2023. It is now read-only.

Fix shopify extension commands timeout when organization has too many apps #1850

Merged
merged 4 commits into from
Dec 17, 2021

Conversation

karreiro
Copy link
Contributor

@karreiro karreiro commented Dec 14, 2021

WHY are these changes introduced?

When users run shopify extension commands, and the organization has too many apps (~ 100), they're facing timeout issues:

error

WHAT is this pull request doing?

Eager loading extensionRegistrations with all_orgs_with_extensions.graphql takes too long for organizations with a large number of apps.

I've initially considered paginating this query to avoid timeouts, but users would still need to wait too long.

So, this PR changes the ShopifyCLI::PartnersAPI::Organizations.fetch_with_extensions method to fetch extensionRegistrations individually and in parallel.

For a small number of apps, this change has almost no impact. But, for many apps (> 40), it gets expressive faster results:

compare

Also, this PR introduces a spinner in the "Loading your extensions…" message.

How to test your changes?

Run shopify extension connect and notice the timeout won't happen anymore*:

compare

It takes some time to fetch 100 apps, so the Gif above demonstrates the command with 3 items. I've double-checked the mechanism locally with more than 100 apps, and I've not faced timeout issues.

Post-release steps

None.

Update checklist

  • I've added a CHANGELOG entry for this PR (if the change is public-facing)
  • I've considered possible cross-platform impacts (Mac, Linux, Windows).
  • I've left the version number as is (we'll handle incrementing this when releasing).
  • I've included any post-release steps in the section above.

@karreiro karreiro requested review from macournoyer, catlee, a team, hannachen and amcaplan and removed request for a team December 14, 2021 08:28
@karreiro karreiro linked an issue Dec 14, 2021 that may be closed by this pull request
lib/shopify_cli/partners_api/organizations.rb Outdated Show resolved Hide resolved
lib/shopify_cli/partners_api/organizations.rb Outdated Show resolved Hide resolved
lib/shopify_cli/partners_api/organizations.rb Outdated Show resolved Hide resolved
lib/shopify_cli/partners_api/organizations.rb Outdated Show resolved Hide resolved
@karreiro
Copy link
Contributor Author

karreiro commented Dec 15, 2021

Thank you for the review and the suggestions, @catlee! 🚀

I've gone through all comments, and I believe we now have a more robust mechanism. Please, let me know what do you think :)

Copy link
Contributor

@catlee catlee left a comment

Choose a reason for hiding this comment

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

Much better, thanks for fixing this up!

Copy link
Contributor

@pepicrft pepicrft left a comment

Choose a reason for hiding this comment

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

This is awesome @karreiro and that pattern that you introduced will be useful in other places. I'll be OOO from tomorrow (Friday) so I won't be able to see the answers to my comments. Don't let them block you, if you disagree with some you can go ahead and merge and we can talk about them in January. Again, amazing!

@karreiro
Copy link
Contributor Author

Thanks a lot for the review, @pepicrft!

I've applied the fixes for most comments in a new commit. Also, don't hesitate to ping me in January if I've missed something 😊

@karreiro karreiro merged commit 41101c3 into main Dec 17, 2021
@karreiro karreiro deleted the fix-1701 branch December 17, 2021 15:09
@gonzaloriestra gonzaloriestra mentioned this pull request Dec 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

extension commands timeout when organization has too many apps
3 participants