Closed
Bug 1221045
Opened 9 years ago
Closed 9 years ago
queue schemata cleanup: i) merge task.yml and create-task-request.yml, ii) correct title in task-reclaim-response to "Task Reclaim Response"
Categories
(Taskcluster :: Services, defect)
Taskcluster
Services
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: pmoore, Assigned: pmoore)
Details
Attachments
(1 file)
Two schema cleanups in this bug.
Cleanup 1: Task Definitions
===========================
Problem description
===================
We currently have a schema for validating task definition inputs to the queue, and a separate one for validating task definition outputs from the queue. The validation is essentially the same, except:
a) For inputs, we provide defaults for values that can be omitted. Since the queue returns all fields, we don't support defaults for validating output from the queue.
b) The property taskGroupId may be omitted as an input, and if the task-graph-scheduler is used, the taskGroupId will be set to the taskId. Therefore it is a "required" field as an output Task Definition from the queue, yet is not a required field as an input.
Reason to change
================
There are two minor cons of using two separate schemas, which is why I am looking to unite them:
A) The two schemata can get out of sync for shared fields. E.g. we see already that retries has a maximum of 50 as an input, but 49 as an output. Also "Task Priority" has no type set for input schemas, but the type "string" for output schemas. These subtle differences are easily introduced when definitions exist in two places.
B) In the generated code for taskcluster clients, we get duplicated code with ugly variable names, e.g. see
* https://github.com/taskcluster/taskcluster-client-go/blob/master/queue/queue.go#L539-L629
* https://github.com/taskcluster/taskcluster-client-go/blob/master/queue/queue.go#L990-L1080
Alternative approach
====================
If we don't want to use exactly the same schema for inputs and outputs, another strategy might be to separate out the commonality of the input and output schemas, and then have two separate schemas for the inputs and outputs that inherit from this common schema. However, this may be a little over-engineered. Open to thoughts on this.
A partial solution could be to change Titles to "Task Definition Request" and "Task Definition Response" at least to avoid that we get structs named "TaskDefinition" and "TaskDefinition1".
Other Considerations
====================
There is not a strong need to have strong validation on output from the Queue, since even if we declare taskGroupId as a field, but not list it as required, it is still legal for the Queue to output it, which it can consistently do.
Cleanup 2: Task Reclaim Response
================================
The title of http://schemas.taskcluster.net/queue/v1/task-reclaim-response.json says "Task Claim Response" whereas it should say "Task Reclaim Response". This is a trivial fix.
Assignee | ||
Comment 1•9 years ago
|
||
Assignee | ||
Comment 2•9 years ago
|
||
I've reworked the implementation for "Cleanup 1" - see PR for details. Long story short, I've gone for the easier option of keeping the definitions duplicated, but cleaning up the differences that leaked in.
It looks like using a single schema is not a good idea, but at the same time inheriting commonality from a separate schema requires huge work for code generation in taskcluster-client-{go,java} which I think isn't worth the effort at least for now.
Instead this solution now is just some simple cleanup...
Comment 3•9 years ago
|
||
Comment on attachment 8682418 [details] [review]
Github Pull Request for taskcluster-queue
Landed...
Attachment #8682418 -
Flags: review?(jopsen) → review+
Comment 4•9 years ago
|
||
THis was some nice cleanups thanks...
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Component: Queue → Services
You need to log in
before you can comment on or make changes to this bug.
Description
•