You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Rajat Khandelwal <ra...@gmail.com> on 2016/06/23 14:58:19 UTC
Review Request 49154: LENS-1202: Add client side iterator for result
in python client
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49154/
-----------------------------------------------------------
Review request for lens.
Bugs: LENS-1202
https://issues.apache.org/jira/browse/LENS-1202
Repository: lens
Description
-------
Diffs
-----
contrib/clients/python/lens/client/models.py 7451fe92b29531f2bdb9219239c6f2b597fe7aac
contrib/clients/python/lens/client/query.py 88ce719890e8bf1c08410284583db14bb23c568c
Diff: https://reviews.apache.org/r/49154/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 49154: LENS-1202: Add client side iterator for
result in python client
Posted by Rajat Khandelwal <ra...@gmail.com>.
> On July 14, 2016, 12:38 p.m., Ajay Yadava wrote:
> > contrib/clients/python/README.md, line 15
> > <https://reviews.apache.org/r/49154/diff/6/?file=1443729#file1443729line15>
> >
> > With all the special methods defined it makes the code unintuitive. You don't know whether to treat 'queries' as a method or as a datastructure.
> >
> > It will be nice to change to something like below:
> >
> > client.get_queries() :returns a list of queries
> > client.get_query() : returns details using query handle.
> >
> > Explicit functions corresponding to various rest endpoints will ensure more readability and intuitive client api.
Valid points, but I wanted to design it a little differently. Will try to explain my thought process in this comment:
1. What you're suggesting would blow up soon since there will be one method for each REST endpoint. And I didn't want to have all those methods in one client instance.
2. Going by that, I decided to have one client for each grouping of endpoints. So `LensClient` as a top-level class doesn't have much functionality, the sub-clients it has are the points of interaction.
3. So the user will be interacting with `client.queryClient`, `client.metadataClient`, `client.logClient`, ...
4. I considered the following options for the syntax of getquery:
4.1. `client.query_client.get_query(handle)`: too much use of `query`
4.2. `client.get_query(handle)`: Because of the first point.
4.3. `client.query_client.get(handle)`: A better version of 4.1, but the final function name is just `get`, which doesn't look nice. The answer to get `what` is only getting conveyed by the `query_client` part in the code.
4.4. Improving 4.3 further, `client.queries.get(handle)` looks much better because `queries` is a better answer to `what` than `query_client`.
4.5. Built-in collection classes(e.g. dict), and other classes that behave like collections(e.g. MailBox) implement a function `get(x, default=None)` and accompany that with `__getitem__`. So I finally decided on using `__getitem__`. `get(x, default)` wouldn't make sense in our case, since a query is either there, or it's not there. No point of defaults.
5. The above is supported when thinking about similar syntax for other clients, e.g. `client.metadata.cubes[name]`, or `client.logs[handle]`.
So `client.queries` is indended to be an object, which can behave as a collection just to make the resulting code look more aligned with the inherent language syntax and features.
So after deciding name for the sub-clients, For search I picked from `client.queries.search` and `client.search_queries`, settling in between at `client.queries`, since it's phonetically closer to `client.search_queries`, and syntactically concise.
- Rajat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49154/#review142196
-----------------------------------------------------------
On July 14, 2016, 11:22 a.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49154/
> -----------------------------------------------------------
>
> (Updated July 14, 2016, 11:22 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1202
> https://issues.apache.org/jira/browse/LENS-1202
>
>
> Repository: lens
>
>
> Description
> -------
>
>
> Diffs
> -----
>
> contrib/clients/python/README.md 00525fffe0c84421ca60cdb23bc9ed7f42963736
> contrib/clients/python/lens/client/main.py dc604979415160ae17c9fcdcef3f258d5e38af38
> contrib/clients/python/lens/client/models.py 7451fe92b29531f2bdb9219239c6f2b597fe7aac
> contrib/clients/python/lens/client/query.py 88ce719890e8bf1c08410284583db14bb23c568c
> contrib/clients/python/lens/client/session.py PRE-CREATION
> contrib/clients/python/test/test_lensclient.py d49c55bdca924eabd0b61f540372db4c3f3e695e
>
> Diff: https://reviews.apache.org/r/49154/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 49154: LENS-1202: Add client side iterator for
result in python client
Posted by Ajay Yadava <aj...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49154/#review142196
-----------------------------------------------------------
contrib/clients/python/README.md (line 15)
<https://reviews.apache.org/r/49154/#comment207760>
With all the special methods defined it makes the code unintuitive. You don't know whether to treat 'queries' as a method or as a datastructure.
It will be nice to change to something like below:
client.get_queries() :returns a list of queries
client.get_query() : returns details using query handle.
Explicit functions corresponding to various rest endpoints will ensure more readability and intuitive client api.
- Ajay Yadava
On July 14, 2016, 5:52 a.m., Rajat Khandelwal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49154/
> -----------------------------------------------------------
>
> (Updated July 14, 2016, 5:52 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1202
> https://issues.apache.org/jira/browse/LENS-1202
>
>
> Repository: lens
>
>
> Description
> -------
>
>
> Diffs
> -----
>
> contrib/clients/python/README.md 00525fffe0c84421ca60cdb23bc9ed7f42963736
> contrib/clients/python/lens/client/main.py dc604979415160ae17c9fcdcef3f258d5e38af38
> contrib/clients/python/lens/client/models.py 7451fe92b29531f2bdb9219239c6f2b597fe7aac
> contrib/clients/python/lens/client/query.py 88ce719890e8bf1c08410284583db14bb23c568c
> contrib/clients/python/lens/client/session.py PRE-CREATION
> contrib/clients/python/test/test_lensclient.py d49c55bdca924eabd0b61f540372db4c3f3e695e
>
> Diff: https://reviews.apache.org/r/49154/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rajat Khandelwal
>
>
Re: Review Request 49154: LENS-1202: Add client side iterator for
result in python client
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49154/
-----------------------------------------------------------
(Updated July 14, 2016, 11:22 a.m.)
Review request for lens.
Bugs: LENS-1202
https://issues.apache.org/jira/browse/LENS-1202
Repository: lens
Description
-------
Diffs (updated)
-----
contrib/clients/python/README.md 00525fffe0c84421ca60cdb23bc9ed7f42963736
contrib/clients/python/lens/client/main.py dc604979415160ae17c9fcdcef3f258d5e38af38
contrib/clients/python/lens/client/models.py 7451fe92b29531f2bdb9219239c6f2b597fe7aac
contrib/clients/python/lens/client/query.py 88ce719890e8bf1c08410284583db14bb23c568c
contrib/clients/python/lens/client/session.py PRE-CREATION
contrib/clients/python/test/test_lensclient.py d49c55bdca924eabd0b61f540372db4c3f3e695e
Diff: https://reviews.apache.org/r/49154/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 49154: LENS-1202: Add client side iterator for
result in python client
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49154/
-----------------------------------------------------------
(Updated June 29, 2016, 2:59 p.m.)
Review request for lens.
Bugs: LENS-1202
https://issues.apache.org/jira/browse/LENS-1202
Repository: lens
Description
-------
Diffs (updated)
-----
contrib/clients/python/README.md 00525fffe0c84421ca60cdb23bc9ed7f42963736
contrib/clients/python/lens/client/main.py dc604979415160ae17c9fcdcef3f258d5e38af38
contrib/clients/python/lens/client/models.py 7451fe92b29531f2bdb9219239c6f2b597fe7aac
contrib/clients/python/lens/client/query.py 88ce719890e8bf1c08410284583db14bb23c568c
contrib/clients/python/lens/client/session.py PRE-CREATION
contrib/clients/python/test/test_lensclient.py d49c55bdca924eabd0b61f540372db4c3f3e695e
Diff: https://reviews.apache.org/r/49154/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 49154: LENS-1202: Add client side iterator for
result in python client
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49154/
-----------------------------------------------------------
(Updated June 28, 2016, 1:37 p.m.)
Review request for lens.
Bugs: LENS-1202
https://issues.apache.org/jira/browse/LENS-1202
Repository: lens
Description
-------
Diffs (updated)
-----
contrib/clients/python/lens/client/main.py dc604979415160ae17c9fcdcef3f258d5e38af38
contrib/clients/python/lens/client/models.py 7451fe92b29531f2bdb9219239c6f2b597fe7aac
contrib/clients/python/lens/client/query.py 88ce719890e8bf1c08410284583db14bb23c568c
contrib/clients/python/lens/client/session.py PRE-CREATION
contrib/clients/python/test/test_lensclient.py d49c55bdca924eabd0b61f540372db4c3f3e695e
Diff: https://reviews.apache.org/r/49154/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 49154: LENS-1202: Add client side iterator for
result in python client
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49154/
-----------------------------------------------------------
(Updated June 28, 2016, 1:22 p.m.)
Review request for lens.
Bugs: LENS-1202
https://issues.apache.org/jira/browse/LENS-1202
Repository: lens
Description
-------
Diffs (updated)
-----
contrib/clients/python/lens/client/main.py dc604979415160ae17c9fcdcef3f258d5e38af38
contrib/clients/python/lens/client/models.py 7451fe92b29531f2bdb9219239c6f2b597fe7aac
contrib/clients/python/lens/client/query.py 88ce719890e8bf1c08410284583db14bb23c568c
contrib/clients/python/lens/client/session.py PRE-CREATION
contrib/clients/python/test/test_lensclient.py d49c55bdca924eabd0b61f540372db4c3f3e695e
Diff: https://reviews.apache.org/r/49154/diff/
Testing
-------
Thanks,
Rajat Khandelwal
Re: Review Request 49154: LENS-1202: Add client side iterator for
result in python client
Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49154/
-----------------------------------------------------------
(Updated June 27, 2016, 5:59 p.m.)
Review request for lens.
Bugs: LENS-1202
https://issues.apache.org/jira/browse/LENS-1202
Repository: lens
Description
-------
Diffs (updated)
-----
contrib/clients/python/lens/client/main.py dc604979415160ae17c9fcdcef3f258d5e38af38
contrib/clients/python/lens/client/models.py 7451fe92b29531f2bdb9219239c6f2b597fe7aac
contrib/clients/python/lens/client/query.py 88ce719890e8bf1c08410284583db14bb23c568c
contrib/clients/python/lens/client/session.py PRE-CREATION
contrib/clients/python/test/test_lensclient.py d49c55bdca924eabd0b61f540372db4c3f3e695e
Diff: https://reviews.apache.org/r/49154/diff/
Testing
-------
Thanks,
Rajat Khandelwal