-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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: Run a job from the Web UI #4592
Conversation
By default, blocks have a margin of 1.5em to create a consistent vertical rhythm. However, sometimes elements need to be associated with the element above them. In this cases, the gap between elements needs to be tighter. There are many ways to do this, but this approach asks the latter content to be marked as associative. The implication is that the association is with the previous block.
This has bit me more than once. It's best just to make Mirage consistent with the API even if it currently means indeterminate job ids
@@ -59,6 +59,36 @@ export default Watchable.extend({ | |||
const url = this.urlForFindRecord(job.get('id'), 'job'); | |||
return this.ajax(url, 'DELETE'); | |||
}, | |||
|
|||
parse(spec) { |
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.
So nice the API does this 💅
export default Controller.extend({ | ||
parseError: null, | ||
planError: null, | ||
runError: null, |
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.
What's the reason for separating these out? Would you ever get a parse error that could then proceed to a plan?
this.setIDByPayload(json); | ||
promise = RSVP.resolve(definition); | ||
} catch (err) { | ||
// If the definition is invalid JSON, assume it is HCL. If it is invalid |
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.
Ha, we do this in Vault too.
@@ -8,6 +8,7 @@ const Router = EmberRouter.extend({ | |||
|
|||
Router.map(function() { | |||
this.route('jobs', function() { | |||
this.route('run'); |
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.
🏃♂️
@import './core/tabs'; | ||
@import './core/tag'; | ||
@import './core/title'; | ||
@import './core/typography'; |
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.
💥Quotepocalypse - did prettier just not do this file before?
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.
Apparently not. It doesn't get touched very often.
<h3 class="title is-4" data-test-run-error-title>Run Error</h3> | ||
<p data-test-run-error-message>{{runError}}</p> | ||
</div> | ||
{{/if}} |
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 I see why the separate errors makes sense now.
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.
I'm still on the fence about whether or not to merge it all into one error. I think it's a wash and I already implemented it this way /shrug
ui/app/templates/jobs/run.hbs
Outdated
</div> | ||
<div class="content is-associative"> | ||
<button class="button is-primary {{if submit.isRunning "is-loading"}}" onclick={{perform submit}} data-test-run>Run</button> | ||
<button class="button is-light" onclick={{action reset}} data-test-cancel>Cancel</button> |
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.
Need some type=button for these two - And not sure if you care about disabling the run button since it's an e-c task, but might be nice to add the disabled
prop.
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.
type=button 🤦♂️ HTML can be ridiculous at times.
Good call on the disabled prop.
return value; | ||
}, | ||
}); | ||
} |
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.
ooo this is nice!
test('when plan fails, the plan error message is shown', function(assert) { | ||
const spec = hclJob(); | ||
|
||
const errorMessage = 'Parse Failed!! :o'; |
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.
lol
JobRun.visit(); | ||
|
||
andThen(() => { | ||
JobRun.editor.fillIn(spec); |
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.
This is still so good, will likely lift this 😄
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.
This is really clean - nice work. I had a couple of questions about buttons (surprise, I know), but 👍 otherwise.
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
This is part of a bigger Web UI Job Writes project
Sometimes you just wanna paste some code and hit the green button, ya know?
Now you will be able to run a job (either HCL or JSON) from the web ui by clicking the "Run Job" button found on the jobs list page.
A quick Q&A before screenshots
Q: Isn't this an Infrastructure as Code anti-pattern?
A: Strictly speaking, yes! Since the UI isn't wired up to source control, it's absolutely possible for your cluster to get out of sync with your source of truth. However, in certain environments it might be easier to submit jobs via the web ui and still commit the job file changes to source control. More commonly, users may want to run jobs in dev or test clusters where infrastructure as code isn't as important to the organization. We're just giving people options 🙂
Q: Oooooh, I don't want this. Where's the off button?
A: Right now the off button is ACLs. Set an anonymous token that only permits read access. Then (without an elevated token) none of this write stuff will work.
Q: This is great! Will I still get to see a plan? And will I get version diffs and deployments?
A: Yes and yes. The Web UI and the CLI are built on top of the same APIs, so everything will work as expected.