-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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
fix: Rename database from 'couchbasedb' to 'couchbase' in documentation and db_engine_specs #29911
Changes from all commits
ceeb621
135f169
a892bab
bf93aaf
a6d889b
8ade0e6
8cf2d4f
bfb6776
3d6682a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,7 +54,7 @@ are compatible with Superset. | |
| [Azure MS SQL](/docs/configuration/databases#sql-server) | `pip install pymssql` | `mssql+pymssql://UserName@presetSQL:[email protected]:1433/TestSchema` | | ||
| [ClickHouse](/docs/configuration/databases#clickhouse) | `pip install clickhouse-connect` | `clickhousedb://{username}:{password}@{hostname}:{port}/{database}` | | ||
| [CockroachDB](/docs/configuration/databases#cockroachdb) | `pip install cockroachdb` | `cockroachdb://root@{hostname}:{port}/{database}?sslmode=disable` | | ||
| [CouchbaseDB](/docs/configuration/databases#couchbaseDB) | `pip install couchbase-sqlalchemy` | `couchbasedb://{username}:{password}@{hostname}:{port}?truststorepath={ssl certificate path}` | | ||
| [Couchbase](/docs/configuration/databases#couchbase) | `pip install couchbase-sqlalchemy` | `couchbase://{username}:{password}@{hostname}:{port}?truststorepath={ssl certificate path}` | | ||
| [Dremio](/docs/configuration/databases#dremio) | `pip install sqlalchemy_dremio` | `dremio://user:pwd@host:31010/` | | ||
| [Elasticsearch](/docs/configuration/databases#elasticsearch) | `pip install elasticsearch-dbapi` | `elasticsearch+http://{user}:{password}@{host}:9200/` | | ||
| [Exasol](/docs/configuration/databases#exasol) | `pip install sqlalchemy-exasol` | `exa+pyodbc://{username}:{password}@{hostname}:{port}/my_schema?CONNECTIONLCALL=en_US.UTF-8&driver=EXAODBC` | | ||
|
@@ -375,9 +375,10 @@ cockroachdb://root@{hostname}:{port}/{database}?sslmode=disable | |
|
||
|
||
|
||
#### CouchbaseDB | ||
#### Couchbase | ||
|
||
The recommended connector library for CouchbaseDB is | ||
The Couchbase's Superset connection is designed to support two services: Couchbase Analytics and Couchbase Columnar. | ||
The recommended connector library for couchbase is | ||
[couchbase-sqlalchemy](https://github.com/couchbase/couchbase-sqlalchemy). | ||
``` | ||
pip install couchbase-sqlalchemy | ||
|
@@ -386,7 +387,7 @@ pip install couchbase-sqlalchemy | |
The expected connection string is formatted as follows: | ||
|
||
``` | ||
couchbasedb://{username}:{password}@{hostname}:{port}?truststorepath={certificate path}?ssl={true/false} | ||
couchbase://{username}:{password}@{hostname}:{port}?truststorepath={certificate path}?ssl={true/false} | ||
``` | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,7 +102,7 @@ | |
"clickhouse": Dialects.CLICKHOUSE, | ||
"clickhousedb": Dialects.CLICKHOUSE, | ||
"cockroachdb": Dialects.POSTGRES, | ||
"couchbasedb": Dialects.MYSQL, | ||
"couchbase": Dialects.MYSQL, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would entail a breaking change, and would break every Couchbase connection created by existing users. At minimum we would need to create an alias for the old dialect name to ensure old connections still work. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have corrected this in new patch. I am adding alias to old name also. I am seeing all checks passed too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't see the alias here - can you check how the alias is implemented in the Postgres spec? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My mistake I thought alias means updating sql_parse , I saw oceanbase and postgres db_engine_specs and similar to that I have added |
||
# "crate": ??? | ||
# "databend": ??? | ||
"databricks": Dialects.DATABRICKS, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this removal intentional? Maybe you meant to add Couchbase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This link is not working. So tests were failing. @villebro is discussing about it in previous conversation. So I removed it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ayush-couchbase it seems the link was failing due to being
http
and nothttps
- could you update that? Thehttps
one works for me at least. After that I think we're good to go.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure , Updated in new patch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, the link checker does not block merging... it just gives us little to-do items as maintainers. I'll open a PR today to sweep up whatever's left at the moment.