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

CLI quickstart: Drop asking for database connection parameters in favor of connection string #16093

Closed
wants to merge 5 commits into from

Conversation

jpizzle34
Copy link

Description

Fixes #16066

Type of Change

  • Bugfix
  • Improvement
  • New Feature
  • Refactor / codestyle updates
  • Other, please describe:

Requirements Checklist

  • New / updated tests are included
  • All tests are passing locally
  • Performed a self-review of the submitted code

If adding a new feature:

  • Documentation was added/updated. PR:

Comment on lines +18 to +22
mysql: [connection_string],
pg: [connection_string],
cockroachdb: [connection_string],
oracledb: [connection_string],
mssql: [connection_string],
Copy link
Member

Choose a reason for hiding this comment

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

Is it common for all these databases to rely on a connection string? From what I understand, Oracle relies on a "connect string" (instead of "connection string"), and for mysql/mssql is still more common to use parameters right? 🤔

(Also lets use camelCase for variable names in the codebase 👍🏻 )

Copy link
Member

Choose a reason for hiding this comment

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

I'd say using separate parameters is still very common among webdevelopers. Some accompanying docs would be needed to instruct users how to build such a connection string for engines that don't provide it on startup like MySQL.

Copy link
Member

Choose a reason for hiding this comment

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

@rijkvanzanten Another thought, what if we kept the separate parameters...but the first parameter is Host or Connection URL and then we detect if it is a connection string skip the rest of the parameters? Allows people to use both ways.

Copy link
Member

Choose a reason for hiding this comment

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

@rijkvanzanten Another thought, what if we kept the separate parameters...but the first parameter is Host or Connection URL and then we detect if it is a connection string skip the rest of the parameters? Allows people to use both ways.

In that case, I'd rather start with a question:

Use:
(•) Connection string
( ) Individual Parameters

But tbh, it feels like we can have smart enough defaults by using connection string for postgres/cockroach/oracle, and parameters for everything else

Copy link
Member

Choose a reason for hiding this comment

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

That makes more sense, however, I wonder how this will affect people used to the default values? With this change won't they have to make their own connection string and there will be no defaults?

Copy link
Author

Choose a reason for hiding this comment

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

Is it common for all these databases to rely on a connection string? From what I understand, Oracle relies on a "connect string" (instead of "connection string"), and for mysql/mssql is still more common to use parameters right? thinking

(Also lets use camelCase for variable names in the codebase 👍🏻 )

Yes you're right. I'll make those updates today :)

Copy link
Author

Choose a reason for hiding this comment

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

@rijkvanzanten Another thought, what if we kept the separate parameters...but the first parameter is Host or Connection URL and then we detect if it is a connection string skip the rest of the parameters? Allows people to use both ways.

In that case, I'd rather start with a question:

Use:
(•) Connection string
( ) Individual Parameters

But tbh, it feels like we can have smart enough defaults by using connection string for postgres/cockroach/oracle, and parameters for everything else

I'm for this. Supporting both connection params and individual params, by asking the question after the user select their DB client.

@jpizzle34 jpizzle34 marked this pull request as draft October 25, 2022 23:28
@BrianRonin
Copy link

BrianRonin commented Nov 13, 2022

I'm not able to connect to cockroachlabs hope this solves it

image

@rijkvanzanten
Copy link
Member

Closing this for now as the suggested solution doesn't cover all use cases. See the thread here! 🙂 #16093 (review)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI Starter create-directus-project: Allow for specifying options param in db connection
5 participants