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

database__connection__filename isn't blank and this causes an error with MySQL #310

Closed
boycottseattle opened this issue May 27, 2022 · 15 comments · Fixed by #323
Closed

Comments

@boycottseattle
Copy link

If you watch the log from the container when loaded, you get this error (multiple times):

Ignoring invalid configuration option passed to Connection: filename. This is currently a warning, but in future versions of MySQL2, an error will be thrown if you pass an invalid configuration option to a Connection

@wglambert
Copy link

Where's that variable from? I don't see it in the image's docs
https://github.com/docker-library/docs/tree/master/ghost#-via-docker-stack-deploy-or-docker-compose

@boycottseattle
Copy link
Author

boycottseattle commented May 27, 2022

It’s an element inside config.production.json that is prefilled by the docker itself. Normally the variable used there is for the internal SQLite database, but it causes problems with mysql - hence the error in the log.

Perhaps the issue here is that it SHOULD be a variable, one that you can leave blank when using mysql.

See this thread on ghost’s forum: https://forum.ghost.org/t/ignoring-invalid-configuration-option-passed-to-connection-filename-this-is-currently-a-warning-but-in-future-versions-of-mysql2-an-error-will-be-thrown-if-you-pass-an-invalid-configuration-option-to-a-connection/30008/18

@timscha
Copy link

timscha commented May 29, 2022

I can confirm the "issue" with a fresh installation with Mysql 8 und Ghost 5.

@tianon
Copy link
Member

tianon commented May 31, 2022

@acburdine I think this filename bit of configuration is generated by Ghost-CLI, right? Do you think that's something we should try to work around in the image somehow? (I'm not sure exactly what to do here. 🙈)

@reraxe
Copy link

reraxe commented Jul 5, 2022

I did a fresh installation to migrate from sqlite3 to MySQL:8.0, and it produced the same error.

Fired up sqlite3 service, and back to normal.

I think it could be the filename of the sqlite3 config?

"database": {
  "client": "sqlite3",
  "connection": {
    "filename": "content/data/ghost-test.db"
  },
  "useNullAsDefault": true,
  "debug": false
}

@ErisDS
Copy link
Collaborator

ErisDS commented Jul 26, 2022

Does the docker image always run in development mode? That would be bad... 😬

As far as I can tell there are only 2 ways this error could happen...

  1. you use a config with sqlite3 and override with mysql somehow (e.g. env vars)
  2. ghost is running in development mode but the default dev config is overridden with mysql config

In the first case - the config should be updated/changed.

In the second case - that's fine if you're running docker for development. The warning is correct but irrelevant. If you're using docker for production, then it should be running in production mode with a production config that has no sqlite3 variables and no filename anywhere.

@tianon
Copy link
Member

tianon commented Jul 27, 2022

We default to NODE_ENV=production, but we also default to sqlite3 (IIRC, that was both for backwards compat and to make the "first timer" experience better?)

You can see our ghost install line here:

installCmd='gosu node ghost install "$GHOST_VERSION" --db sqlite3 --no-prompt --no-stack --no-setup --dir "$GHOST_INSTALL"'; \

and our ghost config lines are also probably relevant:

gosu node ghost config --ip '::' --port 2368 --no-prompt --db sqlite3 --url http://localhost:2368 --dbpath "$GHOST_CONTENT/data/ghost.db"; \
gosu node ghost config paths.contentPath "$GHOST_CONTENT"; \

It sounds like we probably do need to do something different here, but I'm not exactly sure what we should change. 🙈

@ErisDS
Copy link
Collaborator

ErisDS commented Jul 28, 2022

As of Ghost 5.0 we no longer support sqlite3 in production. It won't fail to work, but it's not a supported setup and we expect more advanced / scaled features to cease working over time.

I think if you're defaulting to production that indicates that this setup is intended for production installs, in which case only mysql8 is supported.

Maybe it makes sense two have two different installs, so there is a separate one for development with sqlite3 ?

But the correct solution IMO would be to remove the use of sqlite3 in favour of mysql 8 with the config in production mode 🙂

@blaine07

This comment was marked as resolved.

@tianon
Copy link
Member

tianon commented Aug 3, 2022

Thanks, @ErisDS! That makes perfect sense, and in hindsight we probably should've not defaulted to sqlite3. 😩

Now we have an interesting transition problem where we don't really have a great way to communicate to users that the next version of the image might break their setups (we usually use major version bumps for changes to the defaults that are this level of disruptive, but I'm guessing that given how recent 5.0 is that 6.0 is not coming soon 😅). Perhaps this is a big enough deal that we should just break the default configuration and try to document how folks can get the previous setup back? I'm not sure. 😞

@blaine07
Copy link

blaine07 commented Aug 3, 2022

Yes, my setup has invalid config thing BUT it’s a container in Unraid and I have ZERO idea where to look and start with trying to figure out what causes this error in my setup. 🥹

@ErisDS
Copy link
Collaborator

ErisDS commented Aug 5, 2022

Hi @tianon, this was a documented breaking change in Ghost 5.0, which we gave advance notice of here on this repo: #294.

We have this internal notice that displays in Ghost admin in the case that the env is production and the DB is not mysql8:

CleanShot 2022-08-05 at 12 17 10@2x

We're currently considering whether this should be a Big Red Banner™️ - but either way, if it could be made possible to detect that the site is running with Docker, we could update this text to be specific and point to specific docs if needed.

@tianon
Copy link
Member

tianon commented Aug 8, 2022

Yeah, unfortunately, I think we just miscalculated just how much sqlite3 needed to be deprecated. 😞

We haven't pushed 5.8 yet (docker-library/official-images#12926), so I'm going to propose we use 5.8 to apply this breaking change so we can point broken users straight back to 5.7 to give them an easy escape hatch while they figure out how they're going to migrate to MySQL (or adjust their configuration manually to get back to sqlite3). Thoughts?

@ErisDS
Copy link
Collaborator

ErisDS commented Aug 8, 2022

5.7 is totally broken for anyone running in development mode. It's not our best release 😂 Might be best to push 5.8.1 and target 5.9 as the breaking change as I think 5.8.1 is pretty stable.

@tianon
Copy link
Member

tianon commented Aug 8, 2022

Aha, that's exactly the type of insight I was hoping for ❤️

We'll go ahead with 5.8 then, and we'll work on a draft PR we can merge for 5.9. 👍

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 a pull request may close this issue.

7 participants