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
chore: update instructions for Pinot in connecting to databases #12238
Conversation
Pinot connection fix
@@ -29,7 +29,7 @@ A list of some of the recommended packages. | |||
|[Apache Hive](/docs/databases/hive)|```pip install pyhive```|```hive://hive@{hostname}:{port}/{database}```| | |||
|[Apache Impala](/docs/databases/impala)|```pip install impala```|```impala://{hostname}:{port}/{database}```| | |||
|[Apache Kylin](/docs/databases/kylin)|```pip install kylinpy```|```kylin://<username>:<password>@<hostname>:<port>/<project>?<param1>=<value1>&<param2>=<value2>```| | |||
|[Apache Pinot](/docs/databases/pinot)|```pip install pinotdb```|```pinot+http://CONTROLLER:5436/ query?server=http://CONTROLLER:5983/```| | |||
|[Apache Pinot](/docs/databases/pinot)|```pip install pinotdb```|```pinot+http://BROKER:5436/query?server=http://CONTROLLER:5983/```| |
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.
reading the Pinot docs, it looks like maybe this should be pinot://
instead of pinot+http://
, do you think that change should be made here too?
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.
I strongly second this, we should drop the +http
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.
yes drop the +http
hey @pablo-tech if you can rebase this branch and address the comment, we can merge this! |
@srinify it's rebased, please merge |
hey @pablo-tech it looks like the PR Lint check is failing: https://github.com/apache/superset/pull/12238/checks?check_run_id=1921219267 I've edited the title of the PR this time for you :) For future reference, try to match the format like the error suggests: "chore: update pinot index.mdx" etc |
Superset -> Pinot connection fix
SUMMARY
The pinot Pinot connection instructions mention the CONTROLLER twice, but in one of those it should be the BROKER instead
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
Per the Apache Pinot instructions on integration with Superset here https://docs.pinot.apache.org/integrations/superset
ADDITIONAL INFORMATION
Only impacts the Superset database drivers instructions page https://superset.apache.org/docs/databases/installing-database-drivers