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

Changed data download from srcopsmetrics to pygithub api #19

Merged
merged 5 commits into from
Nov 15, 2022
Merged

Changed data download from srcopsmetrics to pygithub api #19

merged 5 commits into from
Nov 15, 2022

Conversation

oindrillac
Copy link
Contributor

@oindrillac oindrillac commented Nov 11, 2022 •
edited
Loading

In this PR we move away from srcopsmetrics and use the PyGitHub API directly for downloading data.
This PR resolves #9 and lays thr groundwork needed to work on #10

It also resolves the first part of this #11

There are also some structural changes in this PR apart from the above:

  1. Removed repetetive testing code from model inference notebook. We are already testing the trained model in the model training notebook. Since we decided against deploying the model, there is no use of testing the model stored on S3 again with the same test data generated in the model training notebook.
    Kept the "on Action" bits in the model inference notebook.
  2. Removed Ceph communication class into a separate py file which can referenced from the notebooks.
  3. Removed data collection notebook which was not being used in the workflow, as we decided to move to a script for data collection.

@oindrillac oindrillac changed the title [WIP] changed data download from srcopsmetrics to pygithub api Changed data download from srcopsmetrics to pygithub api Nov 11, 2022
@oindrillac
Copy link
Contributor Author

Removing WIP as workflow passed successfully : https://github.com/oindrillac/time-to-merge-tool/actions/runs/3448551555
The PR is ready for review @aakankshaduggal @MichaelClifford

@github-actions
Copy link

Our Model Predicts this PR to be in category [6]

MichaelClifford
MichaelClifford reviewed Nov 15, 2022
View reviewed changes
requirements.txt
git+https://github.com/aicoe-aiops/ocp-ci-analysis.git#egg=src
ipynb==0.5.1
PyGithub==1.57
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove this requirement? Don't we need Pygithub here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah this seems like a rebase issue. Thanks for catching this

MichaelClifford
MichaelClifford approved these changes Nov 15, 2022
View reviewed changes
Copy link
Member

@MichaelClifford MichaelClifford left a comment

Choose a reason for hiding this comment

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

LGTM!

aakankshaduggal
aakankshaduggal approved these changes Nov 15, 2022
View reviewed changes
Copy link
Member

@aakankshaduggal aakankshaduggal left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@aakankshaduggal aakankshaduggal merged commit dad4c80 into redhat-et:main Nov 15, 2022
@github-actions
Copy link

Our Model Predicts this PR to be in category [6]

1 similar comment
@github-actions
Copy link

Our Model Predicts this PR to be in category [6]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aakankshaduggal aakankshaduggal aakankshaduggal approved these changes

@MichaelClifford MichaelClifford MichaelClifford approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use Github API for data collection
3 participants
@oindrillac @MichaelClifford @aakankshaduggal