Closed
Bug 1335920
Opened 8 years ago
Closed 7 years ago
Use json-e to parameterize action tasks
Categories
(Tree Management :: Treeherder, defect)
Tree Management
Treeherder
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: dustin, Assigned: dustin)
References
Details
Attachments
(3 files)
Bug 1332506 specifies a format for action tasks that involves json-e. So we will need to use json-e to create action tasks from that.
Assignee | ||
Comment 1•8 years ago
|
||
Oh, right, this will occur in treeherder.
Component: Task Configuration → Treeherder
Product: Taskcluster → Tree Management
Version: unspecified → ---
Assignee | ||
Comment 2•8 years ago
|
||
The docs in http://gecko.readthedocs.io/en/latest/taskcluster/taskcluster/in-tree-actions.html and http://gecko.readthedocs.io/en/latest/taskcluster/taskcluster/action-spec.html describe roughly how Jonas expects this to work.
Assignee | ||
Comment 3•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → dustin
Assignee | ||
Comment 4•7 years ago
|
||
I'll ship JSON-e 0.9 -- I'm not quite ready to call it 1.0 yet until I'm sure the two implementations match. But we can put 0.9 in tree and call it good enough.
Assignee | ||
Comment 5•7 years ago
|
||
Hammad already shipped 1.0.0 to npmjs, unfortunately. I think we'll need to publish this as 2.0.0.
I'm also having trouble adding it to the @taskcluster org, but I've sent npmjs a support request for that.
Assignee | ||
Comment 6•7 years ago
|
||
Another issue: the action-spec says
4. Replacing any object on the form {$json: value} with the value of JSON.stringify(result) where result is the result of recursive application of rules 1-4 on value.
but in practice the tasks contain `$dump` and treeherder is accepting that name. I think we should add support in json-e to define custom constructs, too, and define that as a custom operator for this case - https://github.com/taskcluster/json-e/issues/145
Assignee | ||
Comment 7•7 years ago
|
||
@jonasfj as an alternative, what do you think about adding a `$dumps` operator as an alias to `$json` and documenting it as deprecated? That would fix the issue that files like https://public-artifacts.taskcluster.net/CYSXEVR2S36DTQtN6T-Erg/0/public/actions.json use `$dumps`, without opening the door to a lot of tight coupling between json-e and the tools that use it.
Flags: needinfo?(jopsen)
Assignee | ||
Comment 8•7 years ago
|
||
It turns out that this work is pretty simple and limited to just replacing the "fake" JSON-e in the treeherder source with the "real" JSON-e, with the $dumps switch described above. There's more actions work going on apart from this, though.
Assignee | ||
Comment 9•7 years ago
|
||
Comment 10•7 years ago
|
||
At this point we don't have many actions, so let's just break the format... and pretend $dumps never existed :)
Flags: needinfo?(jopsen)
Comment 11•7 years ago
|
||
Comment 12•7 years ago
|
||
Commit pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/f19c8f76fa75e6d796c5c6c9b35f5617e2486df0
Bug 1335920 - support both $dumps and $json (#2629)
JSON-e defines `{$json: ..}`, and that's what the actions documentation
says, but Treeherder has been expecting `$dumps`, and the current
`action.json` files use `$dumps`. So this is the first step toward
migrating to the JSON-e standard: make TH accept both. Once this is
deployed, we can switch the in-tree code to generate `$json`. A little
while later, we can switch TH to use JSON-e instead.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 14•7 years ago
|
||
mozreview-review |
Comment on attachment 8886267 [details]
Bug 1335920 - Replace $dumps with $json in action tasks
https://reviewboard.mozilla.org/r/157036/#review162140
Attachment #8886267 -
Flags: review+
Comment 15•7 years ago
|
||
Pushed by dmitchell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/63a62306d326
Replace $dumps with $json in action tasks r=dustin
Comment 16•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•7 years ago
|
Comment 17•7 years ago
|
||
Assignee | ||
Comment 18•7 years ago
|
||
I opened bug 1381622 to update the in-tree docs. So once the treeherder patch here lands, we're good to go.
Comment 19•7 years ago
|
||
Commit pushed to master at https://github.com/mozilla/treeherder
https://github.com/mozilla/treeherder/commit/f616d6b5bb830ad382d6ddb93d7e17784069aab6
Bug 1335920 - Use JSON-e directly to parameterize action tasks (#2636)
This replaces the old, "fake" JSON-e with the real JSON-e library.
The library is packaged as a service since the action spec will
eventually specify additional context values, and we want to apply those
values universally.
Assignee | ||
Comment 20•7 years ago
|
||
Landed, although not deployed yet.
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
Comment 21•7 years ago
|
||
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in
before you can comment on or make changes to this bug.
Description
•