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

[WIP] Add SHOPIFY_API_KEY to DB_PATH when creating database #1061

Closed
wants to merge 2 commits into from

Conversation

mkevinosullivan
Copy link
Contributor

WHY are these changes introduced?

Running the following commands

# create a new app from the Node template
yarn create @shopify/app --template node
# run the app, creating a new app on the partner account
yarn dev

and then opening the provided link will successfully allow the installation of the app.

Doing the following command as a follow-on step

# run the app again, creating a different new app on the partner account
yarn dev --reset

and then opening the provided link will result in a "There's no page at this address" message.

Cause: running the app the second time uses the database.sqlite file created during the first yarn dev, resulting in the error.

WHAT is this pull request doing?

When creating the database file, add the API_KEY in order to differentiate between different instances of the app. The database filename (was database.sqlite) is now db_${API_KEY}.sqlite

@mkevinosullivan mkevinosullivan requested a review from a team as a code owner September 22, 2022 20:53
Copy link
Contributor

@gonzaloriestra gonzaloriestra left a comment

Choose a reason for hiding this comment

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

Thanks! Just two comments:

  • I think there are references to database.sqlite in the README.md
  • We have to do similar changes in the other templates as well, right?

@paulomarg
Copy link
Collaborator

  • I think there are references to database.sqlite in the README.md

Good callout - we might have mentions of this in the tutorials as well.

Copy link
Collaborator

@paulomarg paulomarg left a comment

Choose a reason for hiding this comment

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

LGTM after the points Gonzalo raised!

@gonzaloriestra
Copy link
Contributor

Isn't this ready to be merged?

@mkevinosullivan mkevinosullivan changed the title Add SHOPIFY_API_KEY to DB_PATH when creating database [WIP] Add SHOPIFY_API_KEY to DB_PATH when creating database Dec 14, 2022
@mkevinosullivan
Copy link
Contributor Author

Isn't this ready to be merged?

We haven't merged because we can't do the same implementation across the other templates ... in both the Ruby and PHP templates, the DB must be setup before the first run of yarn dev, as they're managed through Rails or Laravel, respectively.

Hence, we'd end up with an inconsistent experience (there had been a Slack thread on this but it's now long gone)

@gonzaloriestra
Copy link
Contributor

I see, thanks. Any other idea? Does it make sense to try to clear the SQLite database when --reset is run? I know they may use a different database, but I guess we'd cover most cases.

@mkevinosullivan
Copy link
Contributor Author

Closing this as a terrible idea!

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.

None yet

3 participants