Skip to content
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

Add "canceled" build status #438

Merged
merged 3 commits into from
Nov 12, 2024
Merged

Conversation

gabalafou
Copy link
Contributor

@gabalafou gabalafou commented Oct 22, 2024

Description

This PR is the front-end companion to the following server PR:

This pull request:

  • Adds "CANCELED" status to the front end
  • Adds additional status to "Active" build if the build has ended but the status is not "COMPLETED"
    • Rationale: it was weird to me to see a canceled build marked "Active" in the dropdown. Yes, it is the current build, but it's broken and I think this should be a bit more obvious in the UI.

Screenshots

This screenshot shows how a canceled build appears in the UI. Note that this build is not the current build. The dropdown displays "October 22nd, 2024 - 6:19 PM - Canceled." And on the next line we have "Status: Canceled":

This screenshot shows how the current build appears in the dropdown if it was canceled. The line for this build in the dropdown displays "October 22nd, 2024 - 5:36 PM - Active - Canceled":

I want to be clear that I think that this UI is bad. The UI should draw a sharp visual distinction between failed versus successful builds. But the intent of this PR is not to fix the UI, but to at least make it more accurate by reporting a canceled build as canceled rather than "undefined", as Nikita reported.

Pull request checklist

  • Did you test this change locally?
  • [n/a] Did you update the documentation (if required)?
  • Did you add/update relevant tests for this change (if required)?

@peytondmurray peytondmurray self-requested a review October 29, 2024 22:17
@peytondmurray
Copy link
Contributor

I don't have the context to know about this, but how does it work when a build is canceled but still active?

I feel like if it's canceled, the server should activate the last valid build. Not sure about if there are no active builds, but anyway...

Copy link
Contributor

@peytondmurray peytondmurray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - I'm still confused about the design that led to these changes, but I'll leave that question for later.

@peytondmurray peytondmurray added type: enhancement 💅🏼 New feature or request area: UI design 🎨 Items related to the user interface status: merge ready 🚀 labels Oct 29, 2024
@gabalafou
Copy link
Contributor Author

Thanks for the review, @peytonmurray!

You raised good questions.

I don't have the context to know about this, but how does it work when a build is canceled but still active?

This can happen when the first build of an environment is canceled, queued, or failed. If there are no other builds, the server still sets the environment's current_build_id to that build, regardless of whether that build was unsuccessful.

Should I open an issue on the server to return null for the current build id if the environment does not have any successful builds?

Looks good - I'm still confused about the design that led to these changes, but I'll leave that question for later.

There was no design work on this. It's just a fix for the problem described in conda-incubator/conda-store#547.

@gabalafou
Copy link
Contributor Author

By the way, if you're confused about the screenshot in the PR description, showing a build marked "Available" that was built after two "Canceled" builds, that's a bug in the UI. The UI doesn't update the environment metadata after the build has completed. This is the kind of thing that will be fixed in the maintenance refactor.

@gabalafou gabalafou merged commit 8d4785c into conda-incubator:main Nov 12, 2024
6 checks passed
@gabalafou gabalafou deleted the canceled-build branch November 12, 2024 11:35
@peytondmurray
Copy link
Contributor

Should I open an issue on the server to return null for the current build id if the environment does not have any successful builds?

100% in favor of this, please do so. It's super weird to have an active but invalid build. I think either a list element or a red border or some other notifier that indicates <no successful builds> or similar would be more informative to the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: UI design 🎨 Items related to the user interface status: merge ready 🚀 type: enhancement 💅🏼 New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants