-
Notifications
You must be signed in to change notification settings - Fork 84
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
Disable editing for certain fields once bundle reaches starting
state
#4252
Conversation
@percyliang updated |
frontend/src/components/worksheets/BundleDetail/BundleFieldTable/BundleFieldRow.jsx
Outdated
Show resolved
Hide resolved
@percyliang value check updated |
# Request a machine with this much resources and don't let run exceed these resources | ||
# Don't format metadata specs | ||
# fmt: off | ||
METADATA_SPECS.append(MetadataSpec('request_docker_image', str, 'Which docker image (either tag or digest, e.g., codalab/default-cpu:latest) ' |
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 personally think that the original formatting is easier to read and see all the fields at one glance...what do you think?
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.
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.
One option I've been vaguely thinking about is storing the definitions somewhere else. But I don't want to do that in the scope of this PR
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 think putting the definitions here is nice so that everything is in one place. I agree that long lines and weird wrapping is not-ideal either... I only have a weak preference for the single lines, so feel free to wrap if you feel strongly about it.
* @returns {string} joined list values | ||
*/ | ||
export function renderList(list) { | ||
if (!list?.length || !list[0]) { |
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.
Why !list[0]
? In general, I'd like to be a bit more strict about the differences between falsy values...
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.
The tags
field returns ['']
when there aren't any tags. Didn't want to change that in case doing so would break a bunch of stuff, but can change if you think I should.
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.
Oh, I see...maybe write that down as a comment - this is something we should fix. No tags should be []
!
Reasons for making this change
Currently, all bundle fields that aren't generated by CodaLab are editable by the user indefinitely. However, there are certain fields that don't make sense for the user to edit once a bundle has started running, e.g.
store
.That being said, there are some fields that do make sense for the user to edit indefinitely such as
name
anddescription
.This change locks the following editable fields once a bundle has reached the
starting
state:All other editable fields outside of the list above will remain editable indefinitely.
Related issues
#4203
Screenshots
Bundle Before
starting
StateBundle After
starting
StateChecklist