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.

Sign up for GitHub

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

Jump to bottom

Update failure_type_classifier #284

Merged
merged 1 commit into from
Jun 9, 2021
Merged

Update failure_type_classifier #284

merged 1 commit into from
Jun 9, 2021

Conversation

isabelizimm
Copy link
Contributor

@isabelizimm isabelizimm commented Jun 3, 2021 •
edited
Loading

Co-authored-by: Shrey Anand shanand@redhat.com

Related Issues and Dependencies

#274

This introduces a breaking change

  • Yes
  • No

This Pull Request implements

code for running the failure type classification in a pipeline.

Description

  • Changed detect-install-flake to account for the edge case where there aren't any.
  • Changed detect-new-test-failure to account for reference before assignment error.
  • Tested the code with data from these dates: 810, 185, 36, 165, 144, 76, 86
  • Link to running pipeline

Things that can be improved here:

  • Define an output schema based on how it'll be consumed (currently it's a json file)
  • Fix errors for some dashboards and grids (currently it's been skipped)
  • Change output paths based on where we want this stored
  • Dependencies have to be in the same directory which is a limitation of Elyra and so we have two copies of metric_template.ipynb for now. Upstream issue.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@sesheta sesheta requested review from aakankshaduggal and hemajv June 3, 2021 14:00
@sesheta sesheta added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 3, 2021
@isabelizimm isabelizimm changed the title split functions into new notebook, add pipeline, prose Update failure_type_classifier Jun 3, 2021
@isabelizimm isabelizimm changed the title Update failure_type_classifier [WIP]Update failure_type_classifier Jun 3, 2021
@sesheta sesheta added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 3, 2021
@MichaelClifford
Copy link
Member

/retest

MichaelClifford
MichaelClifford reviewed Jun 3, 2021
View reviewed changes
notebooks/failure-type-classification/stage/fail_type_functions.ipynb Outdated
@@ -0,0 +1,343 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Can we move all of these out of stage? and update all the paths accordingly?


Reply via ReviewNB

@review-notebook-app
Copy link

View / edit / reply to this conversation on ReviewNB

MichaelClifford commented on 2021-06-03T20:23:44Z
----------------------------------------------------------------

"the image below"


@review-notebook-app
Copy link

View / edit / reply to this conversation on ReviewNB

MichaelClifford commented on 2021-06-03T20:23:45Z
----------------------------------------------------------------

can the end of this notebook (before the saving to ceph) be to run the failure type classification for all grids?

MichaelClifford
MichaelClifford reviewed Jun 4, 2021
View reviewed changes
notebooks/failure-type-classification/stage/fail_type_functions.ipynb Outdated
" dates = data[tab_name][grid_name][\"timestamps\"]\n",
" infra_flake_dates = np.array(dates)[list([infra_flakes][0][1])]\n",
" infra_flake_dates = [\n",
" datetime.date.fromtimestamp(x // 1000) for x in infra_flake_dates #issue with x here...what is it referring to?\n",
Copy link
Member

Choose a reason for hiding this comment

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

@isabelizimm
I did some testing with pre-commit and I think the issue is with the comment in this line. Removing it seems to fix the issue.

Also, to answer the comment so that it can be removed, this is a list comprehension; works similar to a for loop in the form [ do_something_with_(x) for x in iterator] and returns a list. In this case we are converting items in infra_flake_dates into datetime format from timestamps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahhh yes. When I rearranged the functions I had to update a few things, but forgot I had put that comment in there. When I removed this comment, my pre-commit worked as well. Thank you for doing some detective work!

MichaelClifford
MichaelClifford reviewed Jun 9, 2021
View reviewed changes
notebooks/failure-type-classification/failure_type_classifier.ipynb
@@ -27,7 +27,7 @@
},
Copy link
Member

Choose a reason for hiding this comment

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

Line #7.        filename = f"report_local.json"

Should the locally saved file also have the same day.month.dashboard.job convention?


Reply via ReviewNB

notebooks/failure-type-classification/failure_type_functions.ipynb Show resolved Hide resolved
notebooks/failure-type-classification/metric_template.ipynb Outdated
@@ -0,0 +1,973 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

If we there is no option to call this notebook from its original location, can we customize it to be "helper for failure type" so it is its own notebook? And remove anything that's not needed for the workflow here?


Reply via ReviewNB

notebooks/failure-type-classification/testgrid_flakiness_detection.ipynb
@@ -18,7 +18,7 @@
},
Copy link
Member

Choose a reason for hiding this comment

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

This can be included in the top cell.


Reply via ReviewNB

@isabelizimm isabelizimm force-pushed the failure-classifier branch from 8944f0c to c77188e Compare June 9, 2021 18:33
@isabelizimm isabelizimm changed the title [WIP]Update failure_type_classifier Update failure_type_classifier Jun 9, 2021
@sesheta sesheta removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 9, 2021
@isabelizimm isabelizimm force-pushed the failure-classifier branch from c77188e to 133a443 Compare June 9, 2021 18:38
@isabelizimm
Update failure_type_classifier.ipynb + helper function nb + elyra p…
7bb7060 
…ipeline

Co-authored-by: Shrey Anand <shanand@redhat.com>

Co-authored-by: isabelizimm <izimmerman5298@floridapoly.edu>
@isabelizimm isabelizimm force-pushed the failure-classifier branch from 133a443 to 7bb7060 Compare June 9, 2021 18:41
@MichaelClifford
Copy link
Member

/approve

@sesheta
Copy link
Contributor

sesheta commented Jun 9, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MichaelClifford

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sesheta sesheta added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 9, 2021
@sesheta sesheta merged commit ed47ee6 into aicoe-aiops:master Jun 9, 2021
@MichaelClifford MichaelClifford linked an issue Jun 16, 2021 that may be closed by this pull request
Revisit Test Grid Failure Type Analysis #274
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaelClifford MichaelClifford MichaelClifford left review comments

@durandom durandom Awaiting requested review from durandom durandom is a code owner

@tumido tumido Awaiting requested review from tumido tumido is a code owner

@aakankshaduggal aakankshaduggal Awaiting requested review from aakankshaduggal

@hemajv hemajv Awaiting requested review from hemajv

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Revisit Test Grid Failure Type Analysis
3 participants
@isabelizimm @MichaelClifford @sesheta