Skip to content
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

Have TablesApiRequest Support Additional Query Parameters #437

Merged
merged 5 commits into from
Aug 2, 2017

Conversation

QubitPi
Copy link
Contributor

@QubitPi QubitPi commented Jul 19, 2017

The 2nd milestone of #310

@QubitPi QubitPi force-pushed the getLogicalTableFullView-with-TablesApiRequest branch 2 times, most recently from 6bbe39d to 75782d5 Compare July 19, 2017 21:53
@yahoo yahoo deleted a comment Jul 19, 2017
Copy link
Contributor

@michael-mclawhorn michael-mclawhorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'd like to additionally see in here is actually expanding the TablesApiRequest to load and bind the query parameters (filters, grouping dimensions, etc)

@@ -368,7 +368,12 @@ public Response getTablesFullView(
* @param uriInfo UriInfo of the request
*
* @return Full view of the logical table
*
* @deprecated Inorder to display constrained data availability in table resource, this method needs to accept a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spelling -> In order

@QubitPi QubitPi force-pushed the getLogicalTableFullView-with-TablesApiRequest branch from 1c0561e to 9e2a0e2 Compare July 21, 2017 22:59
@yahoo yahoo deleted a comment Jul 21, 2017
@QubitPi
Copy link
Contributor Author

QubitPi commented Jul 21, 2017

@michael-mclawhorn I moved some methods from DataApiRequest to ApiRequest so that they can be shared between DataApiRequest and TableApiRequest

- [Fili-security module](https://github.com/yahoo/fili/pull/405)
* Added security module for fili data security filters
* Created `ChainingRequestMapper`, and a set of mappers for gatekeeping on security roles and whitelisting dimension filters.
* Created `ChainingRequestMapper`, and a set of mappers for gatekeeping on security roles and whitelisting dimension filters.

- [Add Table-wide Availability](https://github.com/yahoo/fili/pull/414)
* Add `availableIntervals` field to tables endpoint by union the availability for the logical table without taking
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add additional points about moving the methods from DataApiRequest to `ApiRequest'

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to explain the scope of this PR since it is not the complete feature implementation. Otherwise changes looks good.

@yahoo yahoo deleted a comment Jul 25, 2017
@yahoo yahoo deleted a comment Jul 25, 2017
@yahoo yahoo deleted a comment Jul 25, 2017
@yahoo yahoo deleted a comment Jul 25, 2017
@yahoo yahoo deleted a comment Jul 25, 2017
@yahoo yahoo deleted a comment Jul 25, 2017
@yahoo yahoo deleted a comment Jul 25, 2017
@yahoo yahoo deleted a comment Jul 25, 2017
@yahoo yahoo deleted a comment Jul 25, 2017
@yahoo yahoo deleted a comment Jul 25, 2017
@yahoo yahoo deleted a comment Jul 25, 2017
@yahoo yahoo deleted a comment Jul 25, 2017
@yahoo yahoo deleted a comment Jul 25, 2017
@yahoo yahoo deleted a comment Jul 25, 2017
@QubitPi QubitPi force-pushed the getLogicalTableFullView-with-TablesApiRequest branch from c9b578c to 04eff7e Compare July 26, 2017 17:24
@yahoo yahoo deleted a comment Jul 26, 2017
@yahoo yahoo deleted a comment Jul 26, 2017
@yahoo yahoo deleted a comment Jul 26, 2017
@QubitPi QubitPi force-pushed the getLogicalTableFullView-with-TablesApiRequest branch from 02517a8 to c3ecb49 Compare August 2, 2017 23:09
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@yahoo yahoo deleted a comment Aug 2, 2017
@QubitPi QubitPi merged commit b628082 into master Aug 2, 2017
@QubitPi QubitPi deleted the getLogicalTableFullView-with-TablesApiRequest branch October 27, 2017 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants