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

ayush-couchbase · 2024-08-10T13:36:39Z

SUMMARY

Changing Name from couchbasedb to couchbase. Makes it more clear and logical about underlying database.
No major change done.

TESTING INSTRUCTIONS

mannual testing

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

…nd db_engine_specs

github-actions

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️

We hope to see you in our Slack community too! Not signed up? Use our Slack App to self-register.

codecov · 2024-08-10T13:50:06Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.70%. Comparing base (76d897e) to head (3d6682a).
Report is 1094 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #29911       +/-   ##
===========================================
+ Coverage   60.48%   83.70%   +23.21%     
===========================================
  Files        1931      527     -1404     
  Lines       76236    38055    -38181     
  Branches     8568        0     -8568     
===========================================
- Hits        46114    31853    -14261     
+ Misses      28017     6202    -21815     
+ Partials     2105        0     -2105

Flag	Coverage Δ
hive	`49.02% <100.00%> (-0.15%)`	⬇️
javascript	`?`
mysql	`76.76% <100.00%> (?)`
postgres	`76.85% <100.00%> (?)`
presto	`53.56% <100.00%> (-0.24%)`	⬇️
python	`83.70% <100.00%> (+20.21%)`	⬆️
sqlite	`76.34% <100.00%> (?)`
unit	`60.36% <100.00%> (+2.73%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

villebro

There are some issues reported by CI. In addition, the breaking change is problematic, and would need to at least be addressed in UPDATING.md and/or by adding an alias to the old name.

villebro · 2024-08-11T16:33:31Z

superset/sql_parse.py

@@ -102,7 +102,7 @@
    "clickhouse": Dialects.CLICKHOUSE,
    "clickhousedb": Dialects.CLICKHOUSE,
    "cockroachdb": Dialects.POSTGRES,
-    "couchbasedb": Dialects.MYSQL,
+    "couchbase": Dialects.MYSQL,


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.

I have corrected this in new patch. I am adding alias to old name also. I am seeing all checks passed too.

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?

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 .

villebro

Second pass comment

villebro

One last nit, after that this LGTM 👍

villebro · 2024-08-12T16:42:37Z

superset/db_engine_specs/couchbase.py

-    engine = "couchbasedb"
+class CouchbaseEngineSpec(BasicParametersMixin, BaseEngineSpec):
+    engine = "couchbase"
+    engine_aliases = {"couchbase", "couchbasedb"}


You donät need the main name in the alias set, this should be enough:

Suggested change

engine_aliases = {"couchbase", "couchbasedb"}

engine_aliases = {"couchbasedb"}

There are 2 failed checks, but they were not failing earlier, are they because of my patch ?

The failed MySQL test is flaky - we can restart the test. The docs one seems unrelated - It seems resources/INTHEWILD.md has a link to /www.cnovit.com that isn't working. I think we may need to remove the link if the homepage isn't up anymore. Do you want to do it in this PR? I can also open a PR to fix it if needed.

I can fix in this PR. I will upload a new patch.

villebro · 2024-08-12T18:01:51Z

@michael-s-molina this will not be a breaking change as per the last changes. So I think it's safe to remove that label.

rusackas · 2024-08-12T18:53:35Z

RESOURCES/INTHEWILD.md

@@ -80,7 +80,6 @@ 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]


Is this removal intentional? Maybe you meant to add Couchbase?

This link is not working. So tests were failing. @villebro is discussing about it in previous conversation. So I removed it

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

Sure , Updated in new patch

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.

villebro

LGTM, thanks for all the patience and iterations!

rusackas · 2024-10-18T15:10:28Z

FYI @sadpandajoe the author would like to get this into the next release opportunity. I tagged it as 4.1 in case there's an RC4, but let me know if there's anything else I should do to get it into 4.1.1 if RC3 passes the vote.

…on and db_engine_specs (#29911) (cherry picked from commit f5d614d)

Rename connector from 'couchbasedb' to 'couchbase' in documentation a…

ceeb621

…nd db_engine_specs

pull-request-size bot added the size/M label Aug 10, 2024

github-actions bot added the doc Namespace | Anything related to documentation label Aug 10, 2024

dosubot bot added the data:connect Namespace | Anything related to db connections / integrations label Aug 10, 2024

github-actions bot reviewed Aug 10, 2024

View reviewed changes

ayush-couchbase changed the title ~~Rename database from 'couchbasedb' to 'couchbase' in documentation and db_engine_specs~~ fix:Rename database from 'couchbasedb' to 'couchbase' in documentation and db_engine_specs Aug 10, 2024

ayush-couchbase changed the title ~~fix:Rename database from 'couchbasedb' to 'couchbase' in documentation and db_engine_specs~~ fix: Rename database from 'couchbasedb' to 'couchbase' in documentation and db_engine_specs Aug 10, 2024

villebro requested changes Aug 11, 2024

View reviewed changes

ayush-couchbase added 2 commits August 11, 2024 22:30

Updated alias to old name

135f169

Updated alias to old name

a892bab

villebro reviewed Aug 12, 2024

View reviewed changes

ayush-couchbase added 2 commits August 12, 2024 10:25

Updated alias to old name

bf93aaf

Updated alias to old name

a6d889b

villebro reviewed Aug 12, 2024

View reviewed changes

removing couchbase from engine_alias name

8ade0e6

michael-s-molina added risk:breaking-change Issues or PRs that will introduce breaking changes review:checkpoint Last PR reviewed during the daily review standup labels Aug 12, 2024

removing couchbase from engine_alias name

8cf2d4f

michael-s-molina removed the risk:breaking-change Issues or PRs that will introduce breaking changes label Aug 12, 2024

removed link to www.cnovit.com

bfb6776

rusackas reviewed Aug 12, 2024

View reviewed changes

putting cnovit link with https

3d6682a

villebro approved these changes Aug 13, 2024

View reviewed changes

villebro merged commit f5d614d into apache:master Aug 13, 2024
39 checks passed

michael-s-molina removed the review:checkpoint Last PR reviewed during the daily review standup label Aug 13, 2024

rusackas added the v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch label Oct 18, 2024

sadpandajoe pushed a commit that referenced this pull request Nov 13, 2024

fix: Rename database from 'couchbasedb' to 'couchbase' in documentati…

29c76ef

…on and db_engine_specs (#29911) (cherry picked from commit f5d614d)

mistercrunch added 🍒 4.1.1 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels labels Nov 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

ayush-couchbase commented Aug 10, 2024

github-actions bot left a comment

codecov bot commented Aug 10, 2024 •

edited

Loading

villebro left a comment

villebro Aug 11, 2024

ayush-couchbase Aug 11, 2024

villebro Aug 12, 2024

ayush-couchbase Aug 12, 2024

villebro left a comment

villebro left a comment

villebro Aug 12, 2024

ayush-couchbase Aug 12, 2024

ayush-couchbase Aug 12, 2024

villebro Aug 12, 2024

ayush-couchbase Aug 12, 2024

ayush-couchbase Aug 12, 2024

villebro commented Aug 12, 2024

rusackas Aug 12, 2024

ayush-couchbase Aug 12, 2024 •

edited

Loading

villebro Aug 12, 2024

ayush-couchbase Aug 13, 2024

rusackas Oct 18, 2024

villebro left a comment

rusackas commented Oct 18, 2024

	engine_aliases = {"couchbase", "couchbasedb"}
	engine_aliases = {"couchbasedb"}

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

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

Conversation

ayush-couchbase commented Aug 10, 2024

SUMMARY

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

github-actions bot left a comment

Choose a reason for hiding this comment

codecov bot commented Aug 10, 2024 • edited Loading

Codecov Report

villebro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

villebro left a comment

Choose a reason for hiding this comment

villebro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

villebro commented Aug 12, 2024

Choose a reason for hiding this comment

ayush-couchbase Aug 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

villebro left a comment

Choose a reason for hiding this comment

rusackas commented Oct 18, 2024

codecov bot commented Aug 10, 2024 •

edited

Loading

ayush-couchbase Aug 12, 2024 •

edited

Loading