-
Notifications
You must be signed in to change notification settings - Fork 188
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
Task search #839
Task search #839
Conversation
wsorenson
commented
Jan 11, 2016
- needs UX
@ApiParam("Request ID to look up") @PathParam("requestId") String requestId, | ||
@ApiParam("Request ID to match") @PathParam("requestId") String requestId, | ||
@ApiParam("Optional deploy ID to match") @QueryParam("deployId") Optional<String> deployId, | ||
@ApiParam("Optional host to match") @QueryParam("deployId") Optional<String> host, |
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.
just noticed this one, @QueryParam is still deployId
here instead of host
make sure request history always in correct order
revert sorting changes for request history
…e that maybe should be held by the TaskSearch page instead
…k to the form page. Yay! Also some code organizational changes.
…k to the form page. Yay! Also some code organizational changes. And added files that weren't in the previous commit by accident
(WIP) task search efficiency
…Results collections into HistoricalTasks
smarter ordering for queries that don't use zk first
Refactor HistoricalTasks Collections into One
Task Search UI
- `startedBefore`: Optionally match only tasks started before this time (13 digit unix timestamp) | ||
- `orderDirection`: Sort direction (by `startedAt`), can be ASC or DESC, defaults to DESC (newest tasks first) | ||
- `count`: Maximum number of items to return | ||
- `page`: Page of items to view (e.g. page 1 is the first `count` items, page 2 is the next `count` items) |
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.
Count and page are optional too. Page defaults to 1, and I believe count defaults to infinite but I don't remember exactly.
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.
ah, yeah, good to include defaults, I'll add them in
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.
The defaults would be nice to have in api.md too, though it looks like space is limited there. (I can't add line notes to api.md because the diff is so big Github won't show it to me)
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.
Yeah, we need to do some fooling with swagger at some point, that will probably end up being a separate PR some time
👏 |