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

ui: use task state to determine if task is active #14224

Merged
merged 3 commits into from
Aug 23, 2022
Merged

Conversation

lgfa29
Copy link
Contributor

@lgfa29 lgfa29 commented Aug 22, 2022

The current implementation uses the task's finishedAt field to determine
if a task is active of not, but this check is not accurate. A task in
the "pending" state will not have finishedAt value but it's also not
active.

This discrepancy results in some components, like the inline stats chart
of the task row component, to be displayed even whey they shouldn't.

The current implementation uses the task's finishedAt field to determine
if a task is active of not, but this check is not accurate. A task in
the "pending" state will not have finishedAt value but it's also not
active.

This discrepancy results in some components, like the inline stats chart
of the task row component, to be displayed even whey they shouldn't.
@lgfa29 lgfa29 added backport/1.1.x backport to 1.1.x release line backport/1.2.x backport to 1.1.x release line backport/1.3.x backport to 1.3.x release line labels Aug 22, 2022
@lgfa29 lgfa29 added this to the 1.3.4 milestone Aug 22, 2022
@github-actions
Copy link

github-actions bot commented Aug 22, 2022

Ember Asset Size action

As of 434ee99

Files that got Bigger 🚨:

File raw gzip
nomad-ui.js +20 B -10 B

Files that stayed the same size 🤷‍:

File raw gzip
vendor.js 0 B 0 B
nomad-ui.css 0 B 0 B
vendor.css 0 B 0 B

@github-actions
Copy link

github-actions bot commented Aug 22, 2022

Ember Test Audit comparison

main 1542604 change
passes 1410 1400 -10
failures 0 10 +10
flaky 0 0 0
duration 10m 16s 209ms 000ms -10m 16s 209ms

Copy link
Contributor

@ChaiWithJai ChaiWithJai left a comment

Choose a reason for hiding this comment

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

Awesome, awesome job. LGTM.

Comment on lines +60 to +63
@computed('state')
get isActive() {
return this.state === 'running';
}
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: Very smart to map running to active. This will age well!

Comment on lines +190 to +193
const task = server.db.taskStates
.where({ allocationId: allocation.id })
.sortBy('name')[i];
const events = server.db.taskEvents.where({ taskStateId: task.id });
Copy link
Contributor

Choose a reason for hiding this comment

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

praise: excellent use of those Mirage APIs!

Copy link
Contributor

@philrenaud philrenaud left a comment

Choose a reason for hiding this comment

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

lgtm!

@lgfa29
Copy link
Contributor Author

lgfa29 commented Aug 23, 2022

Seems like I broke something. I will fix the tests tomorrow before merge 👍

@lgfa29 lgfa29 merged commit 3953d41 into main Aug 23, 2022
@lgfa29 lgfa29 deleted the ui-fix-task-state branch August 23, 2022 19:50
lgfa29 added a commit that referenced this pull request Aug 23, 2022
The current implementation uses the task's finishedAt field to determine
if a task is active of not, but this check is not accurate. A task in
the "pending" state will not have finishedAt value but it's also not
active.

This discrepancy results in some components, like the inline stats chart
of the task row component, to be displayed even whey they shouldn't.
lgfa29 added a commit that referenced this pull request Aug 23, 2022
The current implementation uses the task's finishedAt field to determine
if a task is active of not, but this check is not accurate. A task in
the "pending" state will not have finishedAt value but it's also not
active.

This discrepancy results in some components, like the inline stats chart
of the task row component, to be displayed even whey they shouldn't.
lgfa29 added a commit that referenced this pull request Aug 23, 2022
The current implementation uses the task's finishedAt field to determine
if a task is active of not, but this check is not accurate. A task in
the "pending" state will not have finishedAt value but it's also not
active.

This discrepancy results in some components, like the inline stats chart
of the task row component, to be displayed even whey they shouldn't.

Co-authored-by: Luiz Aoqui <luiz@hashicorp.com>
lgfa29 added a commit that referenced this pull request Aug 23, 2022
The current implementation uses the task's finishedAt field to determine
if a task is active of not, but this check is not accurate. A task in
the "pending" state will not have finishedAt value but it's also not
active.

This discrepancy results in some components, like the inline stats chart
of the task row component, to be displayed even whey they shouldn't.

Co-authored-by: Luiz Aoqui <luiz@hashicorp.com>
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/1.1.x backport to 1.1.x release line backport/1.2.x backport to 1.1.x release line backport/1.3.x backport to 1.3.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants