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

Build: plan for better description of build state and build status #9110

Closed
6 tasks
agjohnson opened this issue Apr 14, 2022 · 4 comments
Closed
6 tasks

Build: plan for better description of build state and build status #9110

agjohnson opened this issue Apr 14, 2022 · 4 comments
Assignees
Labels
Needed: design decision A core team decision is required

Comments

@agjohnson
Copy link
Contributor

agjohnson commented Apr 14, 2022

I'm breaking this conversation out from #9100, which was around a few buggy sort of issues for build state. We had some great points pop up in #9100 though, but they are longer term and a larger project.

Let's discuss those more here!

Note: we continued discussing this in #9100, so you'll want to review there first

To start, @humitos pointed out a previous discussion, at #7123 (comment):

I'd like to see build state eventually decoupled from build status (or whatever you want to call this concept). How we do this is mostly a matter of modeling, but I see state and status as two competing concepts -- at least as it will apply to our new UX.

I'd describe build state as where in the build process the build is. This allows us to show progression to the user in the form of a progress bar or in the build listing. Our steps now are fairly broad:

* Queued
* Cloning
* Installing
* Building
* Uploading
* Finished

I would describe status as why the build is in a particular state. This is helpful for communicating more details about state to the user, but it doesn't help describe progression. Queued is an important state description, but "Queued" vs "Queue due to build concurrency" is not helpful in describing how far along the build is. Build status would include why a build is queued, why a build was rejected, and error states like why a build failed. Currently, these are all weak concepts in our modeling/UI.

I believe what we are describing so far is consolidating all of the attributes/methods around build status/state/failed/success into these two concepts (this is just a loose approximation):

Build.state in [
	Build.QUEUED,
	Build.CLONING,
	Build.INSTALLING,
	Build.BUILDING,
	Build.UPLOADING,
	Build.COMPLETE,
]
Build.statuses == [
    BuildSuccessfulStatus,
	BuildDuplicatedError,
    BuildCancelledError,
    BuildTimedOutError,
    ...
]

Additional notes

Notes so far on implementation details:

  • The new templates are using both Build model (from templates) and Build APIv3 data (from JS). This means complex logic needs to be duplicated in JS to some degree. It's a huge bonus wherever we can avoid translating logic.
  • It seems we'd need a way to determine which status in Build.statuses is the most important. That is, which to show to the user as the main failure, the top most failure, or which status takes precedence in status display -- for example, if a Build.statuses = [BuildConfigWarning, BuildTimedOutError] or Build.statuses = [BuildConcurrencyRetryStatus, BuildSuccessfulStatus]
  • We should only support a single method for describing build status/state/etc in the templates, but migrating all old Builds also seems ooof
  • Build.status = [BuildSuccessful] is the one I'm not sold on. Though it's a little redundant, Build.success = True` is explicit and easy to query.
  • What is the field type we're talking about? Relation field to another table? I sort of like the idea of making Build.status a JSON field.

Needed

  • So far, we've discusses new build error messages as a separate concept, in Improve build error/warning suggestions #3399. Does it make sense to combine the concept of Build.status and Build.errors as described there?
  • Plan for how to determine if a build is failed, and if a build is successful
  • Plan for what methods/properties/attributes are changing and where they are moving to
  • Plan for how we implement the new pattern on Build and also on the Build API data return. Our templates use the Build model, but we also use the Build APIv3 data return from frontend JS now.
  • Plan for migration?
  • Design doc!
@agjohnson agjohnson added the Needed: design decision A core team decision is required label Apr 14, 2022
@agjohnson agjohnson moved this to Planned in 📍Roadmap Apr 14, 2022
@agjohnson agjohnson changed the title Build: better description of build state and build status Build: plan for better description of build state and build status Apr 14, 2022
@agjohnson
Copy link
Contributor Author

Note: this is just for planning for now, but want at least the discussion on our roadmap. I'm happy to pair on planning, @humitos

@humitos
Copy link
Member

humitos commented Apr 25, 2022

I wrote my thoughts about build's states and build's user-facing messages in #3399 (comment)

@agjohnson
Copy link
Contributor Author

Another note:

It seems build.success is really more problematic that we've discussed so far.

I don't have any ideas for how I'd change the implementation, but if I could only make one change, dropping build.success might be it.

In the conversation about adding a cancelled state, build.success really doesn't make any sense. In fact, build.success only makes sense when the state is finished.

I think a build state "cancelled" makes sense, as it implies a separate final state without a success/failure, much like all the other states do not imply a success/failure.

@humitos
Copy link
Member

humitos commented Oct 31, 2023

I'm not considering dropping build.success since that would affect a lot of places and would require a lot of changes in the logic. The new notification system that I'm designing keeps using the same logic behind build.state (queued, cloning, etc) and build.success.

The main change I'm planning here is that "the build process itself" would be able to attach a new notifications to the build by creating a Notification model instance (for non-error notifications) or by raising an exception (for error notifications) that will stop the build and be handled by the Director (as currently) who will create a Notification model instance for the error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needed: design decision A core team decision is required
Projects
Archived in project
Development

No branches or pull requests

2 participants