jobs: separate description from command#35439
Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom Mar 7, 2019
Merged
Conversation
Member
|
This change is |
6da6ff3 to
604c61e
Compare
madelynnblue
approved these changes
Mar 6, 2019
pkg/ui/src/views/jobs/index.tsx
Outdated
| <div> | ||
| <h3>Command</h3> | ||
| <pre className="job-detail">{job.description}</pre> | ||
| <pre className="job-detail">{job.command ? job.command : job.description}</pre> |
Contributor
There was a problem hiding this comment.
nit: I think this could be simplified to job.command || job.description
604c61e to
65e8dd8
Compare
RaduBerinde
commented
Mar 6, 2019
Member
Author
RaduBerinde
left a comment
There was a problem hiding this comment.
Updated to add the command column to SHOW JOBS as well. CC @knz
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @vilterp)
Member
Author
|
Docs for SHOW JOBS will need to be updated with the |
Contributor
|
The code and overall change LGTM. I do have an issue with the word "command" though. You are introducing a new word without precedent throughout our product. Because you do that we'll have to handle a new type of question: "what's the difference between a command and a SQL statement?" In my opinion, until jobs can be defined to do more things than SQL, we should not use a new word. The word "Statement" or "Query" would be fine. |
Member
Author
|
I used |
Contributor
|
Unless there was a good reason to choose "command" originally (I don't recall such a reason, but perhaps @piyush-singh or @vilterp recall), I'd recommend changing it yes. |
This change adds a `Statement` field to `jobspb.Payload` (and a corresponding `statement` column to `crdb_internal.jobs` and `SHOW JOBS`). The stats job now shows the internal statement in the expanded job view. If the statement isn't present, the description is assumed to be the command. Release note (sql change): SHOW JOBS now returns an extra `statement` column which is populated when the description is not the statement.
65e8dd8 to
f2c15e6
Compare
Member
Author
|
Done - renamed to "statement" everywhere. @vilterp let me know if the UI change is ok with you. |
celiala
approved these changes
Mar 7, 2019
Collaborator
celiala
left a comment
There was a problem hiding this comment.
Synced with @piyush-singh - 👍on using Statement over Command.
Member
Author
|
Thank you! bors r+ |
Contributor
Build failed (retrying...) |
craig bot
pushed a commit
that referenced
this pull request
Mar 7, 2019
35439: jobs: separate description from command r=RaduBerinde a=RaduBerinde This change adds a `Command` field to `jobspb.Payload` (and a corresponding `command` column to `crdb_internal.jobs`). The stats job now shows the internal command in the expanded job view. If the command isn't present, the description is assumed to be the command. Release note: None Example:  Co-authored-by: Radu Berinde <radu@cockroachlabs.com>
Contributor
Build succeeded |
celiala
added a commit
to celiala/cockroach
that referenced
this pull request
Mar 12, 2019
In cockroachdb#35439, we introduced a user-friendly message (instead of the exact SQL statement) for automatic table stats. This commit uses the default font for that message, reserving the monospace font for just SQL text. Release note: None
celiala
added a commit
to celiala/cockroach
that referenced
this pull request
Mar 12, 2019
In cockroachdb#35439, we introduced a user-friendly message (instead of the exact SQL statement) for automatic table stats. This commit uses the default font for that message, reserving the monospace font for just SQL text. Release note: None
craig bot
pushed a commit
that referenced
this pull request
Mar 12, 2019
35607: roachtest: Renenable SQLSmith roachtest r=BramGruneir a=BramGruneir Note that this is the original roachtest and not the re-written one. If we find that this one does not find any different issues, we should remove it entirely. But there is no harm in enabling it right now. Release note: None 35623: storage: enable follower_reads by default r=ajwerner a=ajwerner This PR enables storage level follower reads by default. Follower reads still will not be used for request routing without an enterprise license. Release note: None 35645: sql/sqlbase: fix TestEncDatumSize under go1.12 r=knz a=petermattis Fixes #35636. golang/go@bfc54bb reduced the memory usage of small big integers which in turn reduced the `EncDatum.Size()` for the decimal datum used in this test. Release note: None 35652: ui: remove monospace font for human-readable job description r=celiala a=celiala In #35439, we introduced a user-friendly message (instead of the exact SQL statement) for automatic table stats. This commit uses the default font for that message, reserving the monospace font for just SQL text. cc @rolandcrosby @piyush-singh  35654: roachtest: two easy changefeed test fixes r=nvanbenschoten a=danhhz See commits for details Co-authored-by: Bram Gruneir <bram@cockroachlabs.com> Co-authored-by: Andrew Werner <ajwerner@cockroachlabs.com> Co-authored-by: Peter Mattis <petermattis@gmail.com> Co-authored-by: Celia La <celia@cockroachlabs.com> Co-authored-by: Daniel Harrison <daniel.harrison@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change adds a
Commandfield tojobspb.Payload(and acorresponding
commandcolumn tocrdb_internal.jobs). The stats jobnow shows the internal command in the expanded job view. If the
command isn't present, the description is assumed to be the command.
Release note: None
Example:
