You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "vtlim (via GitHub)" <gi...@apache.org> on 2023/03/15 18:08:27 UTC
[GitHub] [druid] vtlim opened a new pull request, #13938: pip install for Python Druid API
vtlim opened a new pull request, #13938:
URL: https://github.com/apache/druid/pull/13938
This PR makes the `druidapi` pip installable by including a new `setup.py` file, a `druidapi` subdirectory, and updated import statements.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] techdocsmith commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "techdocsmith (via GitHub)" <gi...@apache.org>.
techdocsmith commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1138747282
##########
examples/quickstart/jupyter-notebooks/README.md:
##########
@@ -65,8 +65,15 @@ Make sure you meet the following requirements before starting the Jupyter-based
jupyter notebook --port 3001
```
-- An available Druid instance. You can use the `micro-quickstart` configuration
- described in [Quickstart](https://druid.apache.org/docs/latest/tutorials/index.html).
+- The Python API client for Druid. Clone the Druid repo if you haven't already.
+Go to your Druid source repo and install `druidapi` with the following commands:
+
+ ```bash
+ cd examples/quickstart/jupyter-notebooks/druidapi
+ pip install .
+ ```
+
+- An available Druid instance. You can use the [quickstart deployment](https://druid.apache.org/docs/latest/tutorials/index.html).
Review Comment:
maybe not for this PR, but if we're having them start Juypter in a notebook, we can also give them the option to fire up Druid that way.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vtlim commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "vtlim (via GitHub)" <gi...@apache.org>.
vtlim commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1137652152
##########
examples/quickstart/jupyter-notebooks/druidapi/README.md:
##########
@@ -36,21 +39,25 @@ At present, the best way to use `druidapi` is to clone the Druid repo itself:
git clone git@github.com:apache/druid.git
```
-`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`
+`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`.
+From this directory, install the package and its dependencies with pip using the following command:
-Eventually we would like to create a Python package that can be installed with `pip`. Contributions
-in that area are welcome.
+```
+pip install .
+```
Review Comment:
This is referenced in the previous few lines:
> `druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`.
From this directory, install the package ...
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vtlim commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "vtlim (via GitHub)" <gi...@apache.org>.
vtlim commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1137647530
##########
examples/quickstart/jupyter-notebooks/druidapi/README.md:
##########
@@ -36,21 +39,25 @@ At present, the best way to use `druidapi` is to clone the Druid repo itself:
git clone git@github.com:apache/druid.git
```
-`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`
+`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`.
+From this directory, install the package and its dependencies with pip using the following command:
-Eventually we would like to create a Python package that can be installed with `pip`. Contributions
-in that area are welcome.
+```
+pip install .
+```
-Dependencies are listed in `requirements.txt`.
+Verify your installation by checking that the following commands run in Python without error:
-`druidapi` works against any version of Druid. Operations that exploit newer features obviously work
-only against versions of Druid that support those features.
+```python
+import druidapi
+druid = druidapi.jupyter_client('http://localhost:8888')
+```
-## Getting Started
+## Getting started
To use `druidapi`, first import the library, then connect to your cluster by providing the URL to your Router instance. The way that is done differs a bit between consumers.
-### From a Tutorial Jupyter Notebook
+### From a tutorial Jupyter Notebook
Review Comment:
We use sentence case for headings as per the Google style guide: https://developers.google.com/style/capitalization. Will update `notebook` to lowercase n.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vtlim commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "vtlim (via GitHub)" <gi...@apache.org>.
vtlim commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1137759607
##########
examples/quickstart/jupyter-notebooks/druidapi/setup.py:
##########
@@ -0,0 +1,37 @@
+# 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 setuptools import setup, find_packages
+
+setup(
+ name='druidapi',
+ version='0.1.0',
+ description='Python API client for Apache Druid',
+ url='https://github.com/apache/druid/tree/master/examples/quickstart/jupyter-notebooks/druidapi',
+ author='Paul Rogers',
Review Comment:
Updated
##########
examples/quickstart/jupyter-notebooks/druidapi/README.md:
##########
@@ -36,21 +39,25 @@ At present, the best way to use `druidapi` is to clone the Druid repo itself:
git clone git@github.com:apache/druid.git
```
-`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`
+`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`.
+From this directory, install the package and its dependencies with pip using the following command:
Review Comment:
Updated
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] 317brian commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "317brian (via GitHub)" <gi...@apache.org>.
317brian commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1137843593
##########
examples/quickstart/jupyter-notebooks/README.md:
##########
@@ -65,6 +65,13 @@ Make sure you meet the following requirements before starting the Jupyter-based
jupyter notebook --port 3001
```
+- The Python API client for Druid. From the Druid repo in `examples/quickstart/jupyter-notebooks/druidapi`
Review Comment:
Do we say anywhere that they need to clone the repo first in this file? It says it in the druidapi README
Also `From the Druid repo in ...` sounds kinda awkward although I'm not sure if what I suggested is any better :|
```suggestion
- The Python API client for Druid. In `examples/quickstart/jupyter-notebooks/druidapi` of the Druid repo,
```
##########
examples/quickstart/jupyter-notebooks/README.md:
##########
@@ -65,6 +65,13 @@ Make sure you meet the following requirements before starting the Jupyter-based
jupyter notebook --port 3001
```
+- The Python API client for Druid. From the Druid repo in `examples/quickstart/jupyter-notebooks/druidapi`
+ install `druidapi` with the following command:
+
+ ```bash
+ pip install .
+ ```
+
- An available Druid instance. You can use the `micro-quickstart` configuration
described in [Quickstart](https://druid.apache.org/docs/latest/tutorials/index.html).
Review Comment:
micro-quickstart is gone. I thought we fixed that
```suggestion
- An available Druid instance. You can use the [quickstart deployment](https://druid.apache.org/docs/latest/tutorials/index.html).
```
##########
examples/quickstart/jupyter-notebooks/druidapi/README.md:
##########
@@ -36,21 +39,27 @@ At present, the best way to use `druidapi` is to clone the Druid repo itself:
git clone git@github.com:apache/druid.git
```
-`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`
+`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`.
+From this directory, install the package and its dependencies with pip using the following command:
+
+```
+pip install .
+```
-Eventually we would like to create a Python package that can be installed with `pip`. Contributions
-in that area are welcome.
+Note that there is a second level `druidapi` directory that contains the modules. Do not run
+the install command in this internal folder.
Review Comment:
We usually refer to it as a child or subdirectory and not an internal folder, right?
##########
examples/quickstart/jupyter-notebooks/druidapi/README.md:
##########
@@ -36,21 +39,27 @@ At present, the best way to use `druidapi` is to clone the Druid repo itself:
git clone git@github.com:apache/druid.git
```
-`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`
+`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`.
+From this directory, install the package and its dependencies with pip using the following command:
+
+```
+pip install .
+```
-Eventually we would like to create a Python package that can be installed with `pip`. Contributions
-in that area are welcome.
+Note that there is a second level `druidapi` directory that contains the modules. Do not run
+the install command in this internal folder.
-Dependencies are listed in `requirements.txt`.
+Verify your installation by checking that the following command runs in Python without error:
Review Comment:
```suggestion
Verify your installation by checking that the following command runs in Python:
```
##########
examples/quickstart/jupyter-notebooks/druidapi/README.md:
##########
@@ -36,21 +39,27 @@ At present, the best way to use `druidapi` is to clone the Druid repo itself:
git clone git@github.com:apache/druid.git
```
-`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`
+`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`.
+From this directory, install the package and its dependencies with pip using the following command:
+
+```
+pip install .
+```
-Eventually we would like to create a Python package that can be installed with `pip`. Contributions
-in that area are welcome.
+Note that there is a second level `druidapi` directory that contains the modules. Do not run
+the install command in this internal folder.
-Dependencies are listed in `requirements.txt`.
+Verify your installation by checking that the following command runs in Python without error:
-`druidapi` works against any version of Druid. Operations that exploit newer features obviously work
-only against versions of Druid that support those features.
+```python
+import druidapi
+```
-## Getting Started
+## Getting started
Review Comment:
```suggestion
The import statement should not return anything if it runs successfully.
## Getting started
```
##########
examples/quickstart/jupyter-notebooks/druidapi/README.md:
##########
@@ -28,6 +28,9 @@ in any Python environment, but is optimized for use in Jupyter, providing a comp
environment which complements the UI-based Druid console. The primary use of `druidapi` at present
is to support the set of tutorial notebooks provided in the parent directory.
+`druidapi` works against any version of Druid. Operations that exploit newer features obviously work
Review Comment:
```suggestion
`druidapi` works against any version of Druid. Operations that make use of newer features obviously work
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vtlim commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "vtlim (via GitHub)" <gi...@apache.org>.
vtlim commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1137871731
##########
examples/quickstart/jupyter-notebooks/README.md:
##########
@@ -65,8 +65,14 @@ Make sure you meet the following requirements before starting the Jupyter-based
jupyter notebook --port 3001
```
-- An available Druid instance. You can use the `micro-quickstart` configuration
- described in [Quickstart](https://druid.apache.org/docs/latest/tutorials/index.html).
+- The Python API client for Druid. From the Druid repo in `examples/quickstart/jupyter-notebooks/druidapi`
+ install `druidapi` with the following command:
+
+ ```bash
+ pip install .
Review Comment:
`````suggestion
- The Python API client for Druid. Clone the Druid repo if you haven't already.
Go to your Druid source repo and install `druidapi` with the following commands:
```bash
cd examples/quickstart/jupyter-notebooks/druidapi
pip install .
`````
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vtlim commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "vtlim (via GitHub)" <gi...@apache.org>.
vtlim commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1137649935
##########
examples/quickstart/jupyter-notebooks/README.md:
##########
@@ -65,6 +65,13 @@ Make sure you meet the following requirements before starting the Jupyter-based
jupyter notebook --port 3001
```
+- The Python API client for Druid. From the Druid repo in `examples/quickstart/jupyter-notebooks/druidapi`
+ install `druidapi` with the following command:
+
+ ```bash
+ pip install .
Review Comment:
This is listed in the previous line:
> - The Python API client for Druid. From the Druid repo in `examples/quickstart/jupyter-notebooks/druidapi` install ..
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhishekrb19 commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "abhishekrb19 (via GitHub)" <gi...@apache.org>.
abhishekrb19 commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1140428836
##########
examples/quickstart/jupyter-notebooks/druidapi/setup.py:
##########
@@ -0,0 +1,37 @@
+# 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 setuptools import setup, find_packages
+
+setup(
+ name='druidapi',
+ version='0.1.0',
+ description='Python API client for Apache Druid',
+ url='https://github.com/apache/druid/tree/master/examples/quickstart/jupyter-notebooks/druidapi',
+ author='Apache Druid project',
+ author_email='dev@druid.apache.org',
+ license='Apache License 2.0',
+ packages=find_packages(),
+ install_requires=['requests'],
Review Comment:
One thing to consider when we have more dependencies is to add a separate setup target with the other manually installed dependencies like `jupyterlab`/`notebook`.
Similarly for the [project build dependency](https://druid.apache.org/docs/latest/development/build.html) that requires `pyyaml`.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vtlim commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "vtlim (via GitHub)" <gi...@apache.org>.
vtlim commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1137759882
##########
examples/quickstart/jupyter-notebooks/druidapi/README.md:
##########
@@ -36,21 +39,25 @@ At present, the best way to use `druidapi` is to clone the Druid repo itself:
git clone git@github.com:apache/druid.git
```
-`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`
+`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`.
+From this directory, install the package and its dependencies with pip using the following command:
-Eventually we would like to create a Python package that can be installed with `pip`. Contributions
-in that area are welcome.
+```
+pip install .
+```
-Dependencies are listed in `requirements.txt`.
+Verify your installation by checking that the following commands run in Python without error:
-`druidapi` works against any version of Druid. Operations that exploit newer features obviously work
-only against versions of Druid that support those features.
+```python
+import druidapi
+druid = druidapi.jupyter_client('http://localhost:8888')
Review Comment:
Updated
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vtlim commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "vtlim (via GitHub)" <gi...@apache.org>.
vtlim commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1140445490
##########
examples/quickstart/jupyter-notebooks/README.md:
##########
@@ -65,8 +65,15 @@ Make sure you meet the following requirements before starting the Jupyter-based
jupyter notebook --port 3001
```
-- An available Druid instance. You can use the `micro-quickstart` configuration
- described in [Quickstart](https://druid.apache.org/docs/latest/tutorials/index.html).
+- The Python API client for Druid. Clone the Druid repo if you haven't already.
+Go to your Druid source repo and install `druidapi` with the following commands:
+
+ ```bash
+ cd examples/quickstart/jupyter-notebooks/druidapi
+ pip install .
Review Comment:
Good call, updated
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vtlim commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "vtlim (via GitHub)" <gi...@apache.org>.
vtlim commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1137730623
##########
examples/quickstart/jupyter-notebooks/README.md:
##########
@@ -1,6 +1,6 @@
# Jupyter Notebook tutorials for Druid
-If you are reading this in Jupyter, switch over to the [- START HERE -](- START HERE -.ipynb]
+If you are reading this in Jupyter, switch over to the [- START HERE -](-START%20HERE-.ipynb)
Review Comment:
Renamed start with 0. It'll be easier to work with this file that way also, as opposed to having to do something like `git rm -- -START\ HERE-.ipynb`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vtlim commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "vtlim (via GitHub)" <gi...@apache.org>.
vtlim commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1137872796
##########
examples/quickstart/jupyter-notebooks/README.md:
##########
@@ -65,6 +65,13 @@ Make sure you meet the following requirements before starting the Jupyter-based
jupyter notebook --port 3001
```
+- The Python API client for Druid. From the Druid repo in `examples/quickstart/jupyter-notebooks/druidapi`
Review Comment:
Updated to include clone and cd instructions
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] techdocsmith merged pull request #13938: pip install for Python Druid API
Posted by "techdocsmith (via GitHub)" <gi...@apache.org>.
techdocsmith merged PR #13938:
URL: https://github.com/apache/druid/pull/13938
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vtlim commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "vtlim (via GitHub)" <gi...@apache.org>.
vtlim commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1137640885
##########
examples/quickstart/jupyter-notebooks/druidapi/druidapi/datasource.py:
##########
@@ -32,28 +32,28 @@ class DatasourceClient:
See https://druid.apache.org/docs/latest/operations/api-reference.html#datasources
'''
-
+
Review Comment:
I use vim and configured that in my vimrc 😅
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vtlim commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "vtlim (via GitHub)" <gi...@apache.org>.
vtlim commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1137684110
##########
examples/quickstart/jupyter-notebooks/druidapi/druidapi/__init__.py:
##########
@@ -13,14 +13,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-from .druid import DruidClient
+from druidapi.druid import DruidClient
def jupyter_client(endpoint) -> DruidClient:
'''
Create a Druid client configured to display results as HTML withing a Jupyter notebook.
Waits for the cluster to become ready to avoid intermitent problems when using Druid.
'''
- from .html import HtmlDisplayClient
+ from .html_table import HtmlDisplayClient
Review Comment:
Yes -- I updated the filename to avoid a circular import error:
```
ImportError: cannot import name 'escape' from partially initialized module 'html' (most likely due to a circular import)
```
Will re-update `html_table.py` and `text.py` as we discussed.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vtlim commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "vtlim (via GitHub)" <gi...@apache.org>.
vtlim commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1137853373
##########
examples/quickstart/jupyter-notebooks/druidapi/README.md:
##########
@@ -36,21 +39,27 @@ At present, the best way to use `druidapi` is to clone the Druid repo itself:
git clone git@github.com:apache/druid.git
```
-`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`
+`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`.
+From this directory, install the package and its dependencies with pip using the following command:
+
+```
+pip install .
+```
-Eventually we would like to create a Python package that can be installed with `pip`. Contributions
-in that area are welcome.
+Note that there is a second level `druidapi` directory that contains the modules. Do not run
+the install command in this internal folder.
Review Comment:
```suggestion
the install command in the subdirectory.
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] vtlim commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "vtlim (via GitHub)" <gi...@apache.org>.
vtlim commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1137738344
##########
examples/quickstart/jupyter-notebooks/druidapi/druidapi/__init__.py:
##########
@@ -13,14 +13,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-from .druid import DruidClient
+from druidapi.druid import DruidClient
Review Comment:
Do we have a reason to prefer relative imports? It seems that the [recommended approach](https://peps.python.org/pep-0008/) is absolute imports when possible.
>Absolute imports are recommended, as they are usually more readable and tend to be better behaved (or at least give better error messages) if the import system is incorrectly configured (such as when a directory inside a package ends up on sys.path):
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] paul-rogers commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "paul-rogers (via GitHub)" <gi...@apache.org>.
paul-rogers commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1137771853
##########
examples/quickstart/jupyter-notebooks/druidapi/druidapi/datasource.py:
##########
@@ -32,28 +32,28 @@ class DatasourceClient:
See https://druid.apache.org/docs/latest/operations/api-reference.html#datasources
'''
-
+
Review Comment:
Vim? Now that's true old-school!
##########
examples/quickstart/jupyter-notebooks/druidapi/druidapi/__init__.py:
##########
@@ -13,14 +13,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-from .druid import DruidClient
+from druidapi.druid import DruidClient
Review Comment:
While this may be true of user code, relative imports are often used in libraries. That's where I first saw them. For example, [in Requests](https://github.com/psf/requests/blob/main/requests/__init__.py#L45):
```python
from .exceptions import RequestsDependencyWarning
```
From that same quote above:
> However, explicit relative imports are an acceptable alternative to absolute imports
Also,
> A Foolish Consistency is the Hobgoblin of Little Minds
However, since the code works either way, I suppose it is find to go the verbose route.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] paul-rogers commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "paul-rogers (via GitHub)" <gi...@apache.org>.
paul-rogers commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1137610383
##########
examples/quickstart/jupyter-notebooks/README.md:
##########
@@ -65,6 +65,13 @@ Make sure you meet the following requirements before starting the Jupyter-based
jupyter notebook --port 3001
```
+- The Python API client for Druid. From the Druid repo in `examples/quickstart/jupyter-notebooks/druidapi`
+ install `druidapi` with the following command:
+
+ ```bash
+ pip install .
Review Comment:
What directory must I be in? Should I do this first?
```bash
cd $DRUID_DEV/examples/quickstart/jupyter-notebooks/druidapi
```
##########
examples/quickstart/jupyter-notebooks/README.md:
##########
@@ -1,6 +1,6 @@
# Jupyter Notebook tutorials for Druid
-If you are reading this in Jupyter, switch over to the [- START HERE -](- START HERE -.ipynb]
+If you are reading this in Jupyter, switch over to the [- START HERE -](-START%20HERE-.ipynb)
Review Comment:
Awkward. If the MD tool doesn't handle spaces in names, let's just rename the notebook to `-START-HERE.ipynb`. We could also use '0' as the leading character: anything that sorts before letters.
##########
examples/quickstart/jupyter-notebooks/druidapi/druidapi/__init__.py:
##########
@@ -13,14 +13,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-from .druid import DruidClient
+from druidapi.druid import DruidClient
def jupyter_client(endpoint) -> DruidClient:
'''
Create a Druid client configured to display results as HTML withing a Jupyter notebook.
Waits for the cluster to become ready to avoid intermitent problems when using Druid.
'''
- from .html import HtmlDisplayClient
+ from .html_table import HtmlDisplayClient
Review Comment:
Didn't I move all the display stuff into the `html.py` file as one of my last commits?
##########
examples/quickstart/jupyter-notebooks/druidapi/README.md:
##########
@@ -36,21 +39,25 @@ At present, the best way to use `druidapi` is to clone the Druid repo itself:
git clone git@github.com:apache/druid.git
```
-`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`
+`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`.
+From this directory, install the package and its dependencies with pip using the following command:
-Eventually we would like to create a Python package that can be installed with `pip`. Contributions
-in that area are welcome.
+```
+pip install .
+```
-Dependencies are listed in `requirements.txt`.
+Verify your installation by checking that the following commands run in Python without error:
-`druidapi` works against any version of Druid. Operations that exploit newer features obviously work
-only against versions of Druid that support those features.
+```python
+import druidapi
+druid = druidapi.jupyter_client('http://localhost:8888')
Review Comment:
Only the `import` is needed, which will fail if `druidapi` is not on the Python path.
##########
examples/quickstart/jupyter-notebooks/druidapi/README.md:
##########
@@ -36,21 +39,25 @@ At present, the best way to use `druidapi` is to clone the Druid repo itself:
git clone git@github.com:apache/druid.git
```
-`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`
+`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`.
+From this directory, install the package and its dependencies with pip using the following command:
-Eventually we would like to create a Python package that can be installed with `pip`. Contributions
-in that area are welcome.
+```
+pip install .
+```
Review Comment:
Same note as above.
##########
examples/quickstart/jupyter-notebooks/druidapi/druidapi/__init__.py:
##########
@@ -13,14 +13,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-from .druid import DruidClient
+from druidapi.druid import DruidClient
Review Comment:
These changes are not needed. In Python, '.' means relative to the present module. This allows us to, say, install `druidapi` twice: `druidapi` and `druidApiOld` and have both work.
##########
examples/quickstart/jupyter-notebooks/druidapi/README.md:
##########
@@ -36,21 +39,25 @@ At present, the best way to use `druidapi` is to clone the Druid repo itself:
git clone git@github.com:apache/druid.git
```
-`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`
+`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`.
+From this directory, install the package and its dependencies with pip using the following command:
-Eventually we would like to create a Python package that can be installed with `pip`. Contributions
-in that area are welcome.
+```
+pip install .
+```
-Dependencies are listed in `requirements.txt`.
+Verify your installation by checking that the following commands run in Python without error:
-`druidapi` works against any version of Druid. Operations that exploit newer features obviously work
-only against versions of Druid that support those features.
+```python
+import druidapi
+druid = druidapi.jupyter_client('http://localhost:8888')
+```
-## Getting Started
+## Getting started
To use `druidapi`, first import the library, then connect to your cluster by providing the URL to your Router instance. The way that is done differs a bit between consumers.
-### From a Tutorial Jupyter Notebook
+### From a tutorial Jupyter Notebook
Review Comment:
Always odd that we don't use title case in titles... Is Notebook a proper noun here? Or, should this be:
> From a tutorial Jupyter notebook
##########
examples/quickstart/jupyter-notebooks/druidapi/druidapi/datasource.py:
##########
@@ -32,28 +32,28 @@ class DatasourceClient:
See https://druid.apache.org/docs/latest/operations/api-reference.html#datasources
'''
-
+
Review Comment:
I gotta find the trick in VS Code that drops trailing spaces automatically...
##########
examples/quickstart/jupyter-notebooks/druidapi/README.md:
##########
@@ -36,21 +39,25 @@ At present, the best way to use `druidapi` is to clone the Druid repo itself:
git clone git@github.com:apache/druid.git
```
-`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`
+`druidapi` is located in `examples/quickstart/jupyter-notebooks/druidapi/`.
+From this directory, install the package and its dependencies with pip using the following command:
Review Comment:
Took me a moment to realize you had introduced another directory level. That is helpful: there is not a place to put the test code that I currently have sitting off to the side. Would be good to mention this structure to make it obvious that there there are two `druidapi` directories here.
##########
examples/quickstart/jupyter-notebooks/druidapi/setup.py:
##########
@@ -0,0 +1,37 @@
+# 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 setuptools import setup, find_packages
+
+setup(
+ name='druidapi',
+ version='0.1.0',
+ description='Python API client for Apache Druid',
+ url='https://github.com/apache/druid/tree/master/examples/quickstart/jupyter-notebooks/druidapi',
+ author='Paul Rogers',
Review Comment:
Maybe change this to "Apache Druid project".
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] abhishekrb19 commented on a diff in pull request #13938: pip install for Python Druid API
Posted by "abhishekrb19 (via GitHub)" <gi...@apache.org>.
abhishekrb19 commented on code in PR #13938:
URL: https://github.com/apache/druid/pull/13938#discussion_r1140412650
##########
examples/quickstart/jupyter-notebooks/README.md:
##########
@@ -65,8 +65,15 @@ Make sure you meet the following requirements before starting the Jupyter-based
jupyter notebook --port 3001
```
-- An available Druid instance. You can use the `micro-quickstart` configuration
- described in [Quickstart](https://druid.apache.org/docs/latest/tutorials/index.html).
+- The Python API client for Druid. Clone the Druid repo if you haven't already.
+Go to your Druid source repo and install `druidapi` with the following commands:
+
+ ```bash
+ cd examples/quickstart/jupyter-notebooks/druidapi
+ pip install .
Review Comment:
Not directly related to your change, but we have both `pip` and `pip3` referenced in this README file. It'd be good to just reference one version, perhaps `pip` as python2 is long deprecated now
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org