-
Notifications
You must be signed in to change notification settings - Fork 9
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
Better track ForceBalance Failures #217
Conversation
Co-authored-by: Josh Horton <[email protected]> Co-authored-by: Josh Mitchell <[email protected]>
Worth noting that the most recent version of ForceBalance is pulled down $ wget -O - https://pipelines.actions.githubusercontent.com/serviceHosts/415507c3-0cf5-4512-8746-f9c7b4b67c91/_apis/pipelines/1/runs/2171/signedlogcontent/2\?urlExpires\=2023-01-18T21%3A37%3A43.5258060Z\&urlSigningMethod\=HMACV1\&urlSignature\=GghPzvoDHiYWXVMLR8NPtCXYDaY9KOz63foLKfWIomQ%3D | grep "forcebalance.*conda-forge"
--2023-01-18 15:37:02-- https://pipelines.actions.githubusercontent.com/serviceHosts/415507c3-0cf5-4512-8746-f9c7b4b67c91/_apis/pipelines/1/runs/2171/signedlogcontent/2?urlExpires=2023-01-18T21%3A37%3A43.5258060Z&urlSigningMethod=HMACV1&urlSignature=GghPzvoDHiYWXVMLR8NPtCXYDaY9KOz63foLKfWIomQ%3D
Resolving pipelines.actions.githubusercontent.com (pipelines.actions.githubusercontent.com)... 13.107.42.16
Connecting to pipelines.actions.githubusercontent.com (pipelines.actions.githubusercontent.com)|13.107.42.16|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: unspecified [text/plain]
Saving to: ‘STDOUT’
- [<=> ] 0 --.-KB/s 2023-01-18T21:21:00.6596784Z + forcebalance 1.9.4 py38h507a481_0 conda-forge/linux-64 628kB
2023-01-18T21:22:07.9795057Z forcebalance 1.9.4 py38h507a481_0 conda-forge
2023-01-18T21:22:18.9081378Z forcebalance 1.9.4 py38h507a481_0 conda-forge
- [ <=> ] 238.83K --.-KB/s in 0.09s
2023-01-18 15:37:03 (2.74 MB/s) - written to stdout [244567] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! I think this is the right thing to do for 0.1.4. We should also update the builds of 0.1.x to require forcebalance < 1.9.4
if possible, as pinning the toolkit to <0.11 doesn't seem to be cutting it.
I don't think this is really possible to test in any meaningful way without full integration testing, but if you wanted to create an optimise.err
in the appropriate place and then call this function in a unit test, we could do that. I don't think it's much of a win though - I think the major way this would break is if the name of the error file changed, which isn't local to this function. I'll create an "integration tests!" issue and make sure this is on the list of things to integrationally test.
Description
This implements @jthorton's suggested fix: #216 (comment)
I don't know how to test for this or if any tests are failing on the upstream branch.
Todos
Notable points that this PR has either accomplished or will accomplish.
Questions
Status