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

Make database connection string clearer #3565

Open
wants to merge 3 commits into
base: latest
Choose a base branch
from

Conversation

bytrangle
Copy link

Description

For the connection string to Timescale DB, I enclosed username, password, host and port in angle brackets to indicate to readers that they are not meant to be inserted verbatim and users should add in their own user configs.

Links

N/A

Writing help

For information about style and word usage, see the style guide

Review checklists

Reviewers: use this section to ensure you have checked everything before approving this PR:

Subject matter expert (SME) review checklist

  • Is the content technically accurate?
  • Is the content complete?
  • Is the content presented in a logical order?
  • Does the content use appropriate names for features and products?
  • Does the content provide relevant links to further information?

Documentation team review checklist

  • Is the content free from typos?
  • Does the content use plain English?
  • Does the content contain clear sections for concepts, tasks, and references?
  • Have any images been uploaded to the correct location, and are resolvable?
  • If the page index was updated, are redirects required
    and have they been implemented?
  • Have you checked the built version of this content?

@CLAassistant
Copy link

CLAassistant commented Nov 8, 2024

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Contributor

@billy-the-fish billy-the-fish left a comment

Choose a reason for hiding this comment

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

Nice PR. Suggested the tiny change so the string matches the migration docs: https://docs.timescale.com/migrate/latest/pg-dump-and-restore/.

@@ -108,15 +108,15 @@ ORM (object relational mapper) called [Sequelize][sequelize-info].
1. Compose your connection string variable, using this format:

```java
'postgres://user:pass@example.com:5432/dbname'
'postgres://<user>:<pass>@<host>:<port>/<dbname>'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'postgres://<user>:<pass>@<host>:<port>/<dbname>'
'postgres://<user>:<password>@<host>:<port>/<dbname>'

```

1. Open the `index.js` file you created. Require Sequelize in the application,
and declare the connection string:

```java
const Sequelize = require('sequelize')
const sequelize = new Sequelize('postgres://user:pass@example.com:5432/dbname',
const sequelize = new Sequelize('postgres://<user>:<pass>@<host>:<port>/<dbname>',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const sequelize = new Sequelize('postgres://<user>:<pass>@<host>:<port>/<dbname>',
const sequelize = new Sequelize('postgres://<user>:<password>@<host>:<port>/<dbname>',

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.

3 participants