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

fix: Rename database from 'couchbasedb' to 'couchbase' in documentation and db_engine_specs #29911

Merged
merged 9 commits into from
Aug 13, 2024
Merged

fix: Rename database from 'couchbasedb' to 'couchbase' in documentation and db_engine_specs #29911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ceeb621
Rename connector from 'couchbasedb' to 'couchbase' in documentation a…
ayush-couchbase Aug 10, 2024
135f169
Updated alias to old name
ayush-couchbase Aug 11, 2024
a892bab
Updated alias to old name
ayush-couchbase Aug 11, 2024
bf93aaf
Updated alias to old name
ayush-couchbase Aug 11, 2024
a6d889b
Updated alias to old name
ayush-couchbase Aug 11, 2024
8ade0e6
removing couchbase from engine_alias name
ayush-couchbase Aug 12, 2024
8cf2d4f
removing couchbase from engine_alias name
ayush-couchbase Aug 12, 2024
bfb6776
removed link to www.cnovit.com
ayush-couchbase Aug 12, 2024
3d6682a
putting cnovit link with https
ayush-couchbase Aug 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion RESOURCES/INTHEWILD.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ Join our growing community!
- [Caizin](https://caizin.com/) [@tejaskatariya]
- [Careem](https://www.careem.com/) [@SamraHanifCareem]
- [Cloudsmith](https://cloudsmith.io) [@alancarson]
- [CnOvit](http://www.cnovit.com/) [@xieshaohu]
Copy link
Member

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?

Copy link
Contributor Author

@ayush-couchbase ayush-couchbase Aug 12, 2024 •
edited
Loading

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

Copy link
Member

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 not https - could you update that? The https one works for me at least. After that I think we're good to go.

Copy link
Contributor Author

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

Copy link
Member

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.

- [CnOvit](https://www.cnovit.com/) [@xieshaohu]
- [Cyberhaven](https://www.cyberhaven.com/) [@toliver-ch]
- [Deepomatic](https://deepomatic.com/) [@Zanoellia]
- [Dial Once](https://www.dial-once.com/)
Expand Down
9 changes: 5 additions & 4 deletions docs/docs/configuration/databases.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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` |
Expand Down Expand Up @@ -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
Expand All @@ -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}
```


Expand Down
13 changes: 7 additions & 6 deletions superset/db_engine_specs/couchbasedb.py → superset/db_engine_specs/couchbase.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,15 @@ class CouchbaseParametersSchema(Schema):
)


class CouchbaseDbEngineSpec(BasicParametersMixin, BaseEngineSpec):
engine = "couchbasedb"
class CouchbaseEngineSpec(BasicParametersMixin, BaseEngineSpec):
engine = "couchbase"
engine_aliases = {"couchbasedb"}
engine_name = "Couchbase"
default_driver = "couchbasedb"
default_driver = "couchbase"
allows_joins = False
allows_subqueries = False
sqlalchemy_uri_placeholder = (
"couchbasedb://user:password@host[:port]?truststorepath=value?ssl=value"
"couchbase://user:password@host[:port]?truststorepath=value?ssl=value"
)
parameters_schema = CouchbaseParametersSchema()

Expand Down Expand Up @@ -128,7 +129,7 @@ def build_sqlalchemy_uri(

if parameters.get("port") is None:
uri = URL.create(
"couchbasedb",
"couchbase",
username=parameters.get("username"),
password=parameters.get("password"),
host=parameters["host"],
Expand All @@ -137,7 +138,7 @@ def build_sqlalchemy_uri(
)
else:
uri = URL.create(
"couchbasedb",
"couchbase",
username=parameters.get("username"),
password=parameters.get("password"),
host=parameters["host"],
Expand Down
2 changes: 1 addition & 1 deletion superset/sql_parse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
"clickhouse": Dialects.CLICKHOUSE,
"clickhousedb": Dialects.CLICKHOUSE,
"cockroachdb": Dialects.POSTGRES,
"couchbasedb": Dialects.MYSQL,
"couchbase": Dialects.MYSQL,
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

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

I am adding alias to old name also.

I don't see the alias here - can you check how the alias is implemented in the Postgres spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 engine_aliases = {"couchbase", "couchbasedb"} to db_engine_spec/couchbase.py .

# "crate": ???
# "databend": ???
"databricks": Dialects.DATABRICKS,
Expand Down
12 changes: 6 additions & 6 deletions tests/unit_tests/db_engine_specs/test_couchbase.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,18 @@ def test_epoch_to_dttm() -> None:
"""
DB Eng Specs (couchbase): Test epoch to dttm
"""
from superset.db_engine_specs.couchbasedb import CouchbaseDbEngineSpec
from superset.db_engine_specs.couchbase import CouchbaseEngineSpec

assert CouchbaseDbEngineSpec.epoch_to_dttm() == "MILLIS_TO_STR({col} * 1000)"
assert CouchbaseEngineSpec.epoch_to_dttm() == "MILLIS_TO_STR({col} * 1000)"


def test_epoch_ms_to_dttm() -> None:
"""
DB Eng Specs (couchbase): Test epoch ms to dttm
"""
from superset.db_engine_specs.couchbasedb import CouchbaseDbEngineSpec
from superset.db_engine_specs.couchbase import CouchbaseEngineSpec

assert CouchbaseDbEngineSpec.epoch_ms_to_dttm() == "MILLIS_TO_STR({col})"
assert CouchbaseEngineSpec.epoch_ms_to_dttm() == "MILLIS_TO_STR({col})"


@pytest.mark.parametrize(
Expand All @@ -62,7 +62,7 @@ def test_convert_dttm(
expected_result: Optional[str],
dttm: datetime, # noqa: F811
) -> None:
from superset.db_engine_specs.couchbasedb import CouchbaseDbEngineSpec as spec
from superset.db_engine_specs.couchbase import CouchbaseEngineSpec as spec

assert_convert_dttm(spec, target_type, expected_result, dttm)

Expand All @@ -88,6 +88,6 @@ def test_get_column_spec(
generic_type: GenericDataType,
is_dttm: bool,
) -> None:
from superset.db_engine_specs.couchbasedb import CouchbaseDbEngineSpec as spec
from superset.db_engine_specs.couchbase import CouchbaseEngineSpec as spec

assert_column_spec(spec, native_type, sqla_type, attrs, generic_type, is_dttm)
Loading