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

Feature: Job task counts shows warning if not validated #1476

Merged
merged 2 commits into from
Jun 28, 2018

Conversation

timotheeguerin
Copy link
Member

@timotheeguerin timotheeguerin commented Jun 28, 2018

fix #1475

image

@codecov
Copy link

codecov bot commented Jun 28, 2018

Codecov Report

Merging #1476 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1476      +/-   ##
=========================================
+ Coverage    58.9%   58.9%   +<.01%     
=========================================
  Files        1050    1050              
  Lines       24582   24584       +2     
  Branches     3386    3386              
=========================================
+ Hits        14480   14482       +2     
  Misses      10102   10102
Impacted Files Coverage Δ
...b-progress-status/job-progress-status.component.ts 98.82% <100%> (+0.02%) ⬆️
...k/details/task-timeline/task-timeline.component.ts 89.06% <100%> (ø) ⬆️
app/models/job-task-counts.ts 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fdd2cf4...30376f3. Read the comment docs.

@timotheeguerin timotheeguerin merged commit 3f522a8 into master Jun 28, 2018
expect(warning).toBeFalsy();
});

it("show warning about task count validity if count is unvalidated", () => {

Choose a reason for hiding this comment

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

is there already a warning if the # of tasks > 200000? https://docs.microsoft.com/en-us/azure/batch/batch-get-task-counts

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't, we talked about this at lunch, the problem also is how do we know if there is more than 200k tasks if it can't count it.

The idea was maybe to disable the gauge if it was over 200k

Choose a reason for hiding this comment

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

It is not "can't count" it is "not accurate". I think it is fair to add up the #s and show a warning if the result is >150000. Give that sort of thing some thought.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll append to the warning if total count is close to 200k

Choose a reason for hiding this comment

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

Try to choose a phrase from the feature article. The feature has a very complicated consistency model.

Choose a reason for hiding this comment

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

We can chat in person as well... Ideally the text would match what we surface in the portal.

@timotheeguerin timotheeguerin deleted the feature/task-count-validated branch July 3, 2018 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Task progress not exposing validity of task count api
2 participants