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

get fs-drift to pass CI again #414

Merged
merged 4 commits into from
Mar 17, 2022

Conversation

bengland2
Copy link
Contributor

this is not a real fix, we need to
process response time data and
process counter data into rates over time
but at least the code is structured more reasonably now
JSON parsing is removed from this module
can be fixed later as time and interest permits

@bengland2
Copy link
Contributor Author

ok this is strange error message, all 6 checks above pass but the message is "8 successful and 1 failing check"???

@codecov-commenter
Copy link

Codecov Report

Merging #414 (70621d4) into master (13cb786) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #414   +/-   ##
=======================================
  Coverage   39.35%   39.35%           
=======================================
  Files          15       15           
  Lines         869      869           
=======================================
  Hits          342      342           
  Misses        527      527           
Flag Coverage Δ
gha 39.35% <ø> (ø)
python-3.6 39.35% <ø> (ø)
unit 39.35% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


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 13cb786...70621d4. Read the comment docs.

@bengland2
Copy link
Contributor Author

I have pushed a real fix to fs-drift benchmark-wrapper above (as well as pushing a fix to fs-drift benchmark itself). investigating pre-commit.ci error during build while testing my hand-built image (which built successfully).

@bengland2
Copy link
Contributor Author

The error during build is nothing to do with me, it is CI system not being able to authenticate itself. Please help.

@bengland2
Copy link
Contributor Author

this is ready to merge, has been tested in AWS with fs-drift and OCP 4.9 . The one CI check above that failed is nothing to do with fs-drift. Can someone please merge this?

@bengland2
Copy link
Contributor Author

what did I do wrong to cause CI not to pass? Last night the image build failed with github authentication error.
Screenshot from 2022-03-15 15-27-23

@bengland2
Copy link
Contributor Author

/rerun all

@learnitall
Copy link
Collaborator

Issue lies with our pre-commit config, I'll push a fix here in a second.

@learnitall
Copy link
Collaborator

@bengland2 can you merge in the updated pre-commit config from master?

this is not a real fix, we need to
process response time data and
process counter data into rates over time
but at least the code is structured more reasonably now
JSON parsing is removed from this module
can be fixed later as time permits
have to run each one as an iterator because of yield
@bengland2
Copy link
Contributor Author

@learnitall I rebased my changes and it still fails but for reasons I can do something about now, thank you!

@learnitall
Copy link
Collaborator

Thank you for your patience with the pre-commit issues. TIL to stay up to date on GitHub's blog to stay informed on these sorts of changes.

@learnitall learnitall merged commit 88e8d0a into cloud-bulldozer:master Mar 17, 2022
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.

3 participants