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