Edgetest action

[fdosani]

What

• This PR adds in the edgetest action to ensure the basic requirements are up to date given the tests pass.
• I had to refactor the setup a bit to be PEP517 compliant.

How to Test

• The install locally works for me but maybe a second set of eyes on this would be really helpful @ryanSoley @shania-m

[CLAassistant]

All committers have signed the CLA.

[fdosani]

Whitesource failing isn't a huge surprise. It is using Python 3.7 for the scanning which doesn't support the latest numpy version to remediate the respective findings.

[ryanSoley]

Whitesource failing isn't a huge surprise. It is using Python 3.7 for the scanning which doesn't support the latest numpy version to remediate the respective findings.

whitesource is using python 3.7 or edgetest? we were able to remediate the same whitesource failure by dropping 3.7 in #164

[fdosani]

Whitesource failing isn't a huge surprise. It is using Python 3.7 for the scanning which doesn't support the latest numpy version to remediate the respective findings.

whitesource is using python 3.7 or edgetest? we were able to remediate the same whitesource failure by dropping 3.7 in #164

Whitesource is using 3.7. The edgetest action is using Python 3.9 for its workflow.

If you look at the whitesource run:

numpy-1.21.5-cp37-cp37m-manylinux_2_12_x86_64.manylinux2010_x86_64.whl

I've confirmed with WS that there is no way to configure the bot to use anything but 3.7 at this moment with their free/public offering.

[ryanSoley]

I guess I'm confused as to how I fixed the error in the first place if it was whitesource causing it. #162 and #163 are the same vulnerabilities being reported here in this PR right? #164 fixed them and whitesource never failed after that until now

[fdosani]

@ryanSoley so it seems like WS is unable to use the setup.cfg file format. If you look here in my Rubicon fork, you'll see I only add a space to setup.py to re-trigger WS and it worked. So switching to the setup.cfg style causes it to fail even though they are functionally equivalent .

Edit: That might not be it actually. I have a forked repo where I removed the setup.cfg and it seems to be forcing py 3.7 for some reason: https://github.com/fdosani/datacompy/runs/5487339283

[ryanSoley]

@fdosani that makes sense if it has to do with the file type. on your fork, it may be because the requirements.txt file doesn't specifically say python>=3.8. I only changed the setup on the PR I linked yesterday first and it still failed, then I changed the environment.yml too and it finally stopped erroring

somewhat unrelated - does edgetest only work with setup.cfg, and not setup.py? does that PEP you referenced say people should avoid setup.py?

[fdosani]

@fdosani that makes sense if it has to do with the file type. on your fork, it may be because the requirements.txt file doesn't specifically say python>=3.8. I only changed the setup on the PR I linked yesterday first and it still failed, then I changed the environment.yml too and it finally stopped erroring

somewhat unrelated - does edgetest only work with setup.cfg, and not setup.py? does that PEP you referenced say people should avoid setup.py?

I don't think it says to avoid it explicity. The main this to avoid is all the boiler plate code from my understanding. I know @ak-gupta looked into this PEP a lot more and maybe can comment. It does feel nicer and yes edgetest favours setup.cfg and doesn't edit the setup.py

[fdosani]

@ryanSoley not sure if you are comfortable merging this in or if you wanted to do something different here? I'm not 100% sure why whitesource passed before, but I know this is an issue for our repos and also a couple of other within the c1 org. Happy to discuss further.

[ryanSoley]

@ryanSoley not sure if you are comfortable merging this in or if you wanted to do something different here? I'm not 100% sure why whitesource passed before, but I know this is an issue for our repos and also a couple of other within the c1 org. Happy to discuss further.

is it passing now because we're out of scans? how do you deal with the failures on your repos? do you wait until it runs out of scans and passes by default or just merge over the failed CI?

I'd prefer not to reintroduce the failure and have to deal with it in future builds if its going to block PRs from being merged

[fdosani]

@ryanSoley not sure if you are comfortable merging this in or if you wanted to do something different here? I'm not 100% sure why whitesource passed before, but I know this is an issue for our repos and also a couple of other within the c1 org. Happy to discuss further.

is it passing now because we're out of scans? how do you deal with the failures on your repos? do you wait until it runs out of scans and passes by default or just merge over the failed CI?

I'd prefer not to reintroduce the failure and have to deal with it in future builds if its going to block PRs from being merged

I think we just disposition the finds to say its an issue with WS and then close them. The OSPO is aware of the issue and is working on a alternative solution to try out.

[ryanSoley]

@fdosani have you tried bumping all the 3.7 references in here to 3.8?

https://github.com/capitalone/edgetest/blob/main/setup.cfg

that's essentially how I stopped it from happening on rubicon earlier. since the main setup on rubicon was allowing things to use 3.7 there were deeply nested dependencies that were ending up with 3.7 and the bad numpy version

[ak-gupta]

I don't think it says to avoid it explicity. The main this to avoid is all the boiler plate code from my understanding. I know @ak-gupta looked into this PEP a lot more and maybe can comment. It does feel nicer and yes edgetest favours setup.cfg and doesn't edit the setup.py

I prefer setup.cfg because it's easier to parse programmatically and we can upgrade dependencies in optional installations, not just the base requirements

[ryanSoley]

@shania-m and I just reviewed - looking good! just a few questions

[ryanSoley]

also - if you have a moment to check something out for me - I think I noticed a bug while testing (not one you introduced). intake is a dependency of rubicon, and msgpack is defined as one of their dependencies. when I pip install rubicon, I get intake but not msgpack. if I conda install rubicon I get both. also if I conda install intake on top of the pip installed one I get msgpack. so it seems to me like its a bug in intake's packaging where msgpack is only being installed on conda

all that to say can you try a pip install rubicon-ml and confirm that msgpack is missing, then conda install rubicon-ml and see if its there?

[fdosani]

@ryanSoley request changes have been done, let me know if there is anything else you'd like me to address. Thanks again for the review.

[ryanSoley]

looks great! thanks for adding it in 🎉