-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[ui] Fix: upload jobspec button #23548
Conversation
@@ -15,28 +15,26 @@ | |||
<p> | |||
Paste or author HCL or JSON to submit to your cluster, or select from a list of templates. A plan will be requested before the job is submitted. You can also attach a job spec by uploading a job file or dragging & dropping a file to the editor. | |||
</p> | |||
|
|||
{{#if (can "read variable" path="nomad/job-templates/*" namespace="*")}} |
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.
Psst: review this with "Hide Whitespace" enabled. This change makes it so that you can still upload a jobspec without the "read variables" ACL capability, which I had foolishly wrapped the upload button in before (it being next to the button that actually requires variable read access, "Choose from template")
Ember Test Audit comparison
|
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.
LGTM!
I assume no changelog entry because this hasn't been released yet, right?
3355a01
to
6a41f72
Compare
At some point recently, the upload jobspec button (on the Job Run page) stopped working and I've yet to find a good reason why.
The job editor component uses what's called a render modifier in Ember. It uses a library to help add syntactical sugar to do this.
(Modifier TLDR: it "modifies" an existing HTML element, like a
<div>
in this case, instead of creating a new one in an Ember context. This is useful for codemirror, our text editor, because it needs to exist outside of an Ember context for lifecycle reasons)There's a lifecycle hook in modifiers written this way called
modify()
, and there were previously ones calleddidUpdateArguments()
anddidUpdateArguments()
. Previously, we'd set this up to usedidUpdateArguments
to let thecode-mirror
modifier, but per the library's migrations guide, this is/was being deprecated.Problem is,
modify()
is a lifecycle hook that happens both on initial load and on content update and on external-attribute (like the "should word-wrap" property, etc.) and bundling everything intomodify()
over-compounds this pattern a bit.This PR makes it so that we still use
didUpdateArguments()
, but as a conditional method call frommodify()
.To test:
Head to the jobs index page, click "Run Job", click "Upload", and you should now see the text of your .hcl upload appear in the text editor. In previous (1.8.x anyway) versions this should not work.