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.

Sign up for GitHub

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

Jump to bottom

docs: Update docker-compose.yml example to Postgres 16 and update initialization script #8486

Merged
merged 4 commits into from
Feb 23, 2024
Merged

docs: Update docker-compose.yml example to Postgres 16 and update initialization script #8486

merged 4 commits into from
Feb 23, 2024

Conversation

johnyb0y
Copy link
Contributor

@johnyb0y johnyb0y commented Jan 29, 2024 •
edited
Loading

Summary

Describe what the PR does and how to test. Photos and videos are recommended.

This PR updates the example docker-compose.yml to use Postgres version 16.
Before this change it was Postgres 11, which is EOL since November 9, 2023.

Since the default permissions on the public schema have changed since Postgres 15, we also need to update the included Initialization script. Before, the insecure default was to allow everyone (PUBLIC) to create objects in schema public. Now only the database owner can do that, unless you grant extra privileges. Therefore we need do that for our POSTGRES_NON_ROOT_USER.

I have done a quick test deployment and created some example workflows and found no problems.

Edit: I also looked at the docs and only found a mentioned link to the docker-compose.yml example file in the github repo, which doesn't need to be updated.

Related tickets and issues

Include links to Linear ticket or Github issue or Community forum post. Important in order to close automatically and provide context to reviewers.

Review / Merge checklist

  • PR title and summary are descriptive. Remember, the title automatically goes into the changelog. Use (no-changelog) otherwise. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.

    A bug is not considered fixed, unless a test is added to prevent it from happening again.
    A feature is not complete without tests.

@CLAassistant
Copy link

CLAassistant commented Jan 29, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@n8n-assistant n8n-assistant bot added the community Authored by a community member label Jan 29, 2024
@johnyb0y johnyb0y changed the title Update docker-compose.yml example to Postgres 16 and update initialization script docs: Update docker-compose.yml example to Postgres 16 and update initialization script Jan 29, 2024
@Joffcom
Copy link
Member

Joffcom commented Feb 7, 2024

Hey @johnyb0y,

Thanks for the PR, Would you be able to update the other example that uses Postgres as well?

@johnyb0y
Copy link
Contributor Author

johnyb0y commented Feb 7, 2024

Hey @johnyb0y,

Thanks for the PR, Would you be able to update the other example that uses Postgres as well?

Hi @Joffcom
Sure. I appended the commits on this PR.

@Joffcom
Copy link
Member

Joffcom commented Feb 13, 2024

Thanks @johnyb0y, This looks good to me but just to be safe I would like to ask @netroy to give it a quick review as well, I believe he is out today and will be back tomorrow so another couple of days and this should be good to go.

@johnyb0y
Copy link
Contributor Author

@Joffcom Sure, no problem. Please let me know if I can be of further help here.

netroy
netroy requested changes Feb 14, 2024
View reviewed changes
Copy link
Member

@netroy netroy left a comment

Choose a reason for hiding this comment

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

We should probably add some more info about how to migrate a postgres container from 11 to 16, as the container by default won't run a pg_upgrade.

@johnyb0y
Copy link
Contributor Author

johnyb0y commented Feb 14, 2024 •
edited
Loading

This PR is simply meant as an update to the current docker installation method.
Both docker examples are currently linked in the documentation on how to do a fresh installation of n8n.

I cannot provide upgrade instructions for existing docker installations.

Upgrading an existing database can be non-trivial and things can go wrong (as I'm sure you all know).

@Joffcom
Copy link
Member

Joffcom commented Feb 21, 2024

That is valid, I also think changing database versions is outside of our scope and the user should refere to the documentation provided by their database of choice.

@johnyb0y
Copy link
Contributor Author

Thinking about it some more, maybe we can meet somewhere in the middle.
We could add a general notice somewhere in the docs which refers the user to official instructions on how upgrading a Postgres database works in general.
Regardless of if it is a dockerized database or not.
What do you think?

@netroy
Copy link
Member

netroy commented Feb 21, 2024

@johnyb0y I was thinking the same, and was trying to figure out where to put such a notice to that anyone who uses the compose file directly from this repo can be informed about this change.
What I mean is that if someone pulls the updated compose file, and runs a docker compose create (or something else to update their containers), we need a way to inform them why the setup doesn't work anymore.
I'll take care of this, and will get this PR merged soon.
Sorry for the delays so far.

@johnyb0y
Copy link
Contributor Author

How about this then: I could add a general comment inside of the compose file that the version of postgres needs to be pinned to a major version like "16" and that upgrading an existing Postgres database requires more steps than changing the version number inside the compose file. With a referral to official Postgres upgrade instructions.

If that sounds good to you I could amend this PR.
Otherwise we can go with your suggestion.

netroy
netroy approved these changes Feb 23, 2024
View reviewed changes
Copy link
Member

@netroy netroy left a comment

Choose a reason for hiding this comment

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

We'll take care of updating any docs in a separate PR. let's get this merged.

@netroy netroy merged commit 9b0d984 into n8n-io:master Feb 23, 2024
9 of 10 checks passed
MiloradFilipovic added a commit that referenced this pull request Feb 26, 2024
@MiloradFilipovic
Merge branch 'master' into PAY-1377-send-user-id-on-setup
5f88a18 
* master: (63 commits)
  fix(editor): Fix various typecheck issues (no-changelog) (#8739)
  feat: SQL agent improvements (#8709)
  fix(Postgres Trigger Node): `closeFunction` errors should not prevent a workflow from being deactivated (#8738)
  feat: Add support for Ollama embeddings API (#8732)
  fix(editor): Remove NDVSubConnections component console.log (no-changelog) (#8737)
  fix(editor): Hide previous execution data for sub-nodes in debug mode if it has execution error (#8710)
  feat: Add env variables to support exposing `/workflows/demo` route and `/nodes.json` route (#8506)
  refactor(core): Update auth related typings (no-changelog) (#8686)
  docs: Update docker-compose.yml example to Postgres 16 and update initialization script (#8486)
  fix(editor): Update Filter component state when value is updated (#8684)
  feat(editor): AI Floating Nodes (#8703)
  fix(core): Fix pairedItem issue with partial manual executions (#8575)
  fix: Fix execution error when using AI chain nodes with non-chat model (#8724)
  feat(AI Agent Node): Allow use of Azure Chat model for OpenAI Functions agent (#8725)
  ci: Upgrade `ip` to address CVE-2023-42282 (no-changelog) (#8704)
  fix(core): Ensure `maxRedirects` is used for any http request defining it (#8706)
  fix(Trello Node): Remove GET request body (#8715)
  fix(editor): Do not break NDV for version-less nodes (#8714)
  feat(editor): Retrieve previous chat message on arrow-up (#8696)
  fix(Default Data Loader Node): Fix binary data loader in s3 mode (#8626)
  ...
@janober
Copy link
Member

janober commented Feb 28, 2024

Got released with [email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@netroy netroy netroy approved these changes

Assignees
No one assigned
Labels
community Authored by a community member Released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@johnyb0y @CLAassistant @Joffcom @netroy @janober