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

Add jobs endpoint documentation #29

Merged
merged 1 commit into from
Sep 12, 2016
Merged

Conversation

venisa
Copy link

@venisa venisa commented Aug 31, 2016

Add documentation for Jobs endpoint.

Please note:
I have provided a very brief description for async. However, the focus of this PR is to document the Jobs endpoint.

@cdeszaq @archolewa you guys had reviewed this when this was opened in bard.
I addressed all outstanding comments and added a new commit for filters on jobs endpoint

@venisa venisa added the WIP label Aug 31, 2016
"status": ONE OF ["pending", "success", "error"],
"jobTicket": "TICKET",
"dateCreated": "ISO 8601 DATETIME",
"userId": "Foo"
Copy link
Author

Choose a reason for hiding this comment

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

@archolewa This does not include dateUpdated as our job payload does not currently give dateUpdated. (It's not a part of the job payload in the design docs)

@venisa
Copy link
Author

venisa commented Aug 31, 2016

@cdeszaq @archolewa Please review

@venisa
Copy link
Author

venisa commented Sep 6, 2016

@archolewa Added dateUpdated

@@ -561,6 +563,100 @@ There are, however, a few catches to this:
- Sorting is only applied at the Druid level. Therefore, the results of a sort are not guaranteed to be accurate if Fili
performs any post-Druid calculations on one of the metrics that you are sorting on.

Asynchronous Queries
----
Copy link
Contributor

Choose a reason for hiding this comment

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

The --- should be the same length as the title.

@archolewa
Copy link
Contributor

Finished my latest pass. A bit more about the filters (a link to the data filter section will go a long way) would be nice.

If no jobs are available in the system, we get an empty collection.

The `jobs` endpoint supports the `filters` query param that filters the JobRows according to the filter condition and
only returns the JobRows that satisfy the filter condition.
Copy link
Contributor

@archolewa archolewa Sep 7, 2016

Choose a reason for hiding this comment

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

I think I would rephrase this to make it clearer what we can filter on:

The `jobs` endpoint supports filtering on job fields (i.e. `userId`, `status`), using the same syntax as the [data endpoint filters](#filtering). For example:

userId-eq[greg, joan], status-eq[success]

resolves into the following boolean operation:

(userId = greg OR userId = joan) AND status = success

which will return only those Jobs created by `greg` and `joan` that have completed successfully.

Copy link
Collaborator

Choose a reason for hiding this comment

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

To add to @archolewa comments a bit: the core idea here is to tie it back to the filters that users are already familiar with, and leave the terminology centered around "jobs" since that's what they are interacting with. JobRow is an internal concept, really, so we don't want to expose that in the API if we don't have to

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, add a link in there back up to where we talk about filters in general

@archolewa
Copy link
Contributor

Made a small suggestion to hopefully make the filter documentation a bit clearer. 👍 Once that's been addressed.


1. Providing the status of tickets queried via the `jobs/TICKET` resource.
2. Provide a list of all jobs in the system.
3. Providing access to the results via the `jobs/TICKET/results` resource.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be good to have these in order of coarsest to finest resource (ie. /jobs, /jobs/TICKET, and /jobs/TICKET/results)

/NotABlocker

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could turn each of these into links into the section that talks about it below.

/NotABlocker

@cdeszaq
Copy link
Collaborator

cdeszaq commented Sep 7, 2016

Just a couple final small things and this looks good

@venisa
Copy link
Author

venisa commented Sep 7, 2016

@archolewa @cdeszaq all comments have been addressed

1. Provide a list of [all jobs](#get-ting-summary-of-all-jobs) in the system.
2. Providing the status of a [particular job](#get-ting-job-status) queried via the `jobs/TICKET` resource.
3. Providing access to the [results](#get-ting-results) via the `jobs/TICKET/results` resource.

Copy link
Contributor

Choose a reason for hiding this comment

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

#1 should probably also be Providing.

@cdeszaq
Copy link
Collaborator

cdeszaq commented Sep 8, 2016

👍 after the few final small tweaks

@michael-mclawhorn
Copy link
Contributor

👍 Seems fine.

@venisa venisa force-pushed the Add-Jobs-endpoint-documentation branch from 9d0bcd1 to 7fcfbcc Compare September 8, 2016 20:51
@venisa venisa force-pushed the Add-Jobs-endpoint-documentation branch from 7fcfbcc to 19873c9 Compare September 8, 2016 20:53
@venisa venisa removed the NEED SQUASH label Sep 8, 2016
@cdeszaq cdeszaq force-pushed the Add-Jobs-endpoint-documentation branch from 19873c9 to c58f9e6 Compare September 12, 2016 14:44
@cdeszaq cdeszaq merged commit c58f9e6 into master Sep 12, 2016
@cdeszaq cdeszaq deleted the Add-Jobs-endpoint-documentation branch September 12, 2016 15:09
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.

4 participants