Enable Theme Kit Access passwords#2558
Conversation
AFAIK Shopify would like to reserve "Shop" for the Shop product so I'd recommend sticking to the store convention. |
pepicrft
left a comment
There was a problem hiding this comment.
Tophatted and works as expected 👏🏼
There was a problem hiding this comment.
@pepicrft thanks for the review! Regarding your suggestion about SHOPIFY_SHOP, while I agree it's not the best name, it was already being used for the login (so dropping it would be a breaking change):
So we had two variables for the same thing: SHOPIFY_SHOP and SHOPIFY_CLI_STORE. I decided to remove the latter because it's only used by the CLI3 and we can change it.
What do you think?
Since it's already being used and it's a breaking change I agree. In CLI 3.0 we can make sure we align with the new naming convention. Thanks for clarifying it 🙏🏼 |
karreiro
left a comment
There was a problem hiding this comment.
Thank you, @gonzaloriestra! Great stuff! The code looks really elegant :)
The theme list command works fine, however, I've noticed the theme push gets freezed.
I wonder if the backend of the Theme Kit Access maybe doesn't support the /assets/bulk.json requests (that upload many assets in a single request).
|
@karreiro good catch! Sorry, I hadn't tested all the use cases carefully yet... All the theme commands should work properly now (except the hot reload with I've finally disabled the bulk mode when pushing with a Theme Access password until I find out how to fix it (I've created an issue: https://github.com/Shopify/shopify-cli-planning/issues/367). But it seems to be some kind of limitation of the Theme Access API. |
There was a problem hiding this comment.
Thank you for the fix and for raising the issue, @gonzaloriestra! Awesome stuff 🚀 :)
|
If I'm reading this correctly, does this mean we can finally use Shopify CLI v2.x for proper theme deployment in a CI/CD environment?? 🙏 🙏 |
|
@curiouscrusher yes, it does! 🙂 |
|
This is amazing, thank you very much @gonzaloriestra for adding this much needed functionality! 🎉 🚀 |
WHY are these changes introduced?
First step for https://github.com/Shopify/shopify-cli-planning/issues/332
Fixes https://github.com/Shopify/shopify-cli-planning/issues/365
WHAT is this pull request doing?
SHOPIFY_CLI_ADMIN_AUTH_TOKENcontains a Theme Kit Access password, the theme requests will go through Theme Kit Access API instead of Admin API, so the password is validated. As the login is not required, it also requires passing the store URL throughSHOPIFY_SHOP.SHOPIFY_CLI_STOREtoSHOPIFY_SHOP, which was already existing. It was added recently to be used from the CLI3. I'll update the CLI3 as well to useSHOPIFY_SHOP.How to test your changes?
Check that theme commands work the same way when logged in (by using the Admin API):
bin/shopify loginDEBUG=1 bin/shopify theme listCheck that they also work with a Theme Kit Access password (by using Theme Access Kit API):
bin/shopify logoutDEBUG=1 SHOPIFY_SHOP=your-shop.myshopify.com SHOPIFY_CLI_ADMIN_AUTH_TOKEN=your-password bin/shopify theme listUpdate checklist