Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fix job status aggregation logic#35000

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
lafriks merged 8 commits intogo-gitea:mainfromNorthRealm:patch6-1
Jul 17, 2025
Merged

Fix job status aggregation logic#35000

lafriks merged 8 commits intogo-gitea:mainfromNorthRealm:patch6-1
Jul 17, 2025

Conversation

@NorthRealm
Copy link
Contributor

@NorthRealmNorthRealm commentedJul 8, 2025
edited
Loading

For a run (assume 2 jobs) that has a failed job and a waiting job, the run status should be waiting,as the run is not done yet.

Related#34823

@GiteaBotGiteaBot added the lgtm/need 2This PR needs two approvals by maintainers to be considered for merging. labelJul 8, 2025
@github-actionsgithub-actionsbot added the modifies/goPull requests that update Go code labelJul 8, 2025
@NorthRealm
Copy link
ContributorAuthor

NorthRealm commentedJul 8, 2025
edited
Loading

Before patch:

1

@lunny
Copy link
Member

I think#34823 (comment) can resolve the problem.

@NorthRealm
Copy link
ContributorAuthor

NorthRealm commentedJul 8, 2025
edited
Loading

I think#34823 (comment) can resolve the problem.

No.. The picture is what I just observed 🤦‍♂️ (before patch)

I personally see no need to introduce another run status

@NorthRealm
Copy link
ContributorAuthor

@lunny And for a failed job and a running job as in#34823 , running takes precedence over failure, because the run is not done yet. It's the same thought as for waiting status.

@lunny
Copy link
Member

However, the aggregation logic differs between running/waiting and failure states in two key ways:
1.Failure takes precedence: If any step fails, the overall status should be marked as failure, regardless of the state of other steps.
2.Stopping after failure: If a step fails while others are still running, the system should allow stopping the remaining steps. However, once the overall status is marked as failure, it may no longer be possible to issue a stop.

To address this, I see two possible solutions:
1.Introduce a new aggregation status: running with failure. This status would indicate that some steps are still running while at least one has failed. It would only apply at the aggregate level and not be used for individual steps. I personally prefer this one.
2.Separate aggregation statuses for display and control logic: One status would be used for UI display, and another for cancellation logic. The downside is that the UI might show failure even while some steps are still running, making it unclear whether stopping is still an option.

@NorthRealm
Copy link
ContributorAuthor

NorthRealm commentedJul 8, 2025
edited
Loading

  • Yeah surely any failed Job = Run fail. I'm just worrying that current aggregation could possibly break something, and what I just observed before patch (as in the picture) is the case.
  • If a job fails while others are still running/in waiting state, the system should allow stopping/cancelling remaining jobs.
  • There's already an issue in backend where run marked as failure before all jobs finish. Actually surprised that doesn't spam emails. (Send email on Workflow Run Success/Failure #34982)
    RelatedRerun job only when run is done #34970

@NorthRealmNorthRealm marked this pull request as draftJuly 8, 2025 17:42
@NorthRealmNorthRealm deleted the patch6-1 branchJuly 8, 2025 18:25
@NorthRealmNorthRealm restored the patch6-1 branchJuly 14, 2025 05:39
@GiteaBotGiteaBot added lgtm/need 1This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2This PR needs two approvals by maintainers to be considered for merging. labelsJul 15, 2025
@NorthRealmNorthRealm marked this pull request as ready for reviewJuly 15, 2025 04:26
@GiteaBotGiteaBot added lgtm/doneThis PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1This PR needs approval from one additional maintainer to be merged. labelsJul 17, 2025
@lafrikslafriks merged commit39f145a intogo-gitea:mainJul 17, 2025
26 checks passed
@GiteaBotGiteaBot added this to the1.25.0 milestoneJul 17, 2025
@NorthRealmNorthRealm deleted the patch6-1 branchJuly 17, 2025 18:12
zjjhot added a commit to zjjhot/gitea that referenced this pull requestJul 18, 2025
* giteaofficial/main:  [skip ci] Updated translations via Crowdin  Increase gap on latest commit (go-gitea#35104)  Fix job status aggregation logic (go-gitea#35000)  Fix some missed GitHeadRefName when renaming (go-gitea#35102)  Fix error logs and improve some comments/messages (go-gitea#35105)  Support Basic Authentication for archive downloads (go-gitea#35087)  Run `uv run` with `--frozen` (go-gitea#35097)  Improve package API log handling (go-gitea#35100)  Rename pull request GetGitRefName to GetGitHeadRefName (go-gitea#35093)  Fix review comment/dimiss comment x reference can be refereced back (go-gitea#35094)  Fix submodule nil check (go-gitea#35096)  Redirect to a presigned URL of HEAD for HEAD requests (go-gitea#35088)
@go-giteago-gitea locked asresolvedand limited conversation to collaboratorsOct 15, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@lunnylunnylunny approved these changes

@lafrikslafrikslafriks approved these changes

+1 more reviewer

@a1012112796a1012112796a1012112796 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

lgtm/doneThis PR has enough approvals to get merged. There are no important open reservations anymore.modifies/goPull requests that update Go code

Projects

None yet

Milestone

1.25.0

Development

Successfully merging this pull request may close these issues.

5 participants

@NorthRealm@lunny@lafriks@a1012112796@GiteaBot

[8]ページ先頭

©2009-2025 Movatter.jp