You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/01/29 09:02:20 UTC
[GitHub] [incubator-superset] derari opened a new pull request #9043: Add
support for Cockroach DB
derari opened a new pull request #9043: Add support for Cockroach DB
URL: https://github.com/apache/incubator-superset/pull/9043
### CATEGORY
Choose one
- [ ] Bug Fix
- [x] Enhancement (new features, refinement)
- [ ] Refactor
- [ ] Add tests
- [ ] Build / Development Environment
- [ ] Documentation
### SUMMARY
Adds support for Cockroach DB.
I could not test the change becaues the container always fails with
```
superset_1 | standard_init_linux.go:211: exec user process caused "no such file or directory"
```
but this seems unrelated to my change.
So far I could use Cockroach DB in superset even without explicit support, only the time grain options were missing.
I verified that the time grain options declared by PostgresEngineSpec work with Cockroach DB.
### TEST PLAN
Add a Cockroach DB datasource, add tables and test charts.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #9032
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [x] Introduces new feature or API
- [ ] Removes existing feature or API
### REVIEWERS
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #9043: Add
support for Cockroach DB
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9043: Add support for Cockroach DB
URL: https://github.com/apache/incubator-superset/pull/9043#issuecomment-580136603
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9043?src=pr&el=h1) Report
> Merging [#9043](https://codecov.io/gh/apache/incubator-superset/pull/9043?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/a4ed0758b0f04d6eb4d5da4d81867fb38c1e22ca?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9043?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9043 +/- ##
==========================================
+ Coverage 59.43% 59.45% +0.01%
==========================================
Files 369 369
Lines 11743 11747 +4
Branches 2884 2888 +4
==========================================
+ Hits 6980 6984 +4
Misses 4584 4584
Partials 179 179
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9043?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/assets/src/components/TableSelector.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9043/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL1RhYmxlU2VsZWN0b3IuanN4) | `78.35% <0%> (+0.32%)` | :arrow_up: |
| [...uperset/assets/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9043/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9kYXNoYm9hcmQvY29tcG9uZW50cy9IZWFkZXIuanN4) | `42.06% <0%> (+0.93%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9043?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9043?src=pr&el=footer). Last update [a4ed075...1eb114d](https://codecov.io/gh/apache/incubator-superset/pull/9043?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request
#9043: Add support for Cockroach DB
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #9043: Add support for Cockroach DB
URL: https://github.com/apache/incubator-superset/pull/9043#discussion_r372437045
##########
File path: superset/db_engine_specs/cockroachdb.py
##########
@@ -0,0 +1,32 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from datetime import datetime
+from typing import List, Optional, Tuple, TYPE_CHECKING
+
+from pytz import _FixedOffset # type: ignore
+from sqlalchemy.dialects.postgresql.base import PGInspector
+
+from superset.db_engine_specs.base import LimitMethod
+from superset.db_engine_specs.postgres import PostgresEngineSpec
+
+if TYPE_CHECKING:
+ # prevent circular imports
+ from superset.models.core import Database # pylint: disable=unused-import
Review comment:
You seem to be importing lots of things that aren't being used in this file. If you check CI logs you will see that this is failing builds.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro merged pull request #9043: Add
support for Cockroach DB
Posted by GitBox <gi...@apache.org>.
villebro merged pull request #9043: Add support for Cockroach DB
URL: https://github.com/apache/incubator-superset/pull/9043
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request
#9043: Add support for Cockroach DB
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #9043: Add support for Cockroach DB
URL: https://github.com/apache/incubator-superset/pull/9043#discussion_r372435898
##########
File path: requirements-dev.txt
##########
@@ -16,6 +16,7 @@
#
black==19.3b0
coverage==4.5.3
+cockroachdb==0.3.3
Review comment:
This needs to be moved to `extras_require` in `setup.py`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] derari commented on a change in pull request
#9043: Add support for Cockroach DB
Posted by GitBox <gi...@apache.org>.
derari commented on a change in pull request #9043: Add support for Cockroach DB
URL: https://github.com/apache/incubator-superset/pull/9043#discussion_r372885473
##########
File path: superset/db_engine_specs/cockroachdb.py
##########
@@ -0,0 +1,32 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from datetime import datetime
+from typing import List, Optional, Tuple, TYPE_CHECKING
+
+from pytz import _FixedOffset # type: ignore
+from sqlalchemy.dialects.postgresql.base import PGInspector
+
+from superset.db_engine_specs.base import LimitMethod
+from superset.db_engine_specs.postgres import PostgresEngineSpec
+
+if TYPE_CHECKING:
+ # prevent circular imports
+ from superset.models.core import Database # pylint: disable=unused-import
Review comment:
file is cleaned
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ynohat commented on issue #9043: Add support
for Cockroach DB
Posted by GitBox <gi...@apache.org>.
ynohat commented on issue #9043: Add support for Cockroach DB
URL: https://github.com/apache/incubator-superset/pull/9043#issuecomment-583645752
@derari thanks a lot for your work on this!
I tried to apply the changes in this PR manually, and it didn't work OOTB with docker-compose. It seems like adding the cockroach dep to `setup.py` is not sufficient, it also needs to be added to `docker/requirements-extra.txt`.
This is first contact with superset for me, so forgive me if this comment is incorrect or misguided!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] derari commented on a change in pull request
#9043: Add support for Cockroach DB
Posted by GitBox <gi...@apache.org>.
derari commented on a change in pull request #9043: Add support for Cockroach DB
URL: https://github.com/apache/incubator-superset/pull/9043#discussion_r372885153
##########
File path: requirements-dev.txt
##########
@@ -16,6 +16,7 @@
#
black==19.3b0
coverage==4.5.3
+cockroachdb==0.3.3
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org