Code cleanup
๐ฃ Description
Refactoring code base to better follow python PEP-8 convention to improve future maintainability.
๐ญ Motivation and Context
Following issue #14, the code base should more strictly follow Python convention to hopefully improve maintainability.
๐งช Testing
Since no new functionality was added to the project, we used the same tests available in the repository to verify proper functionality. Changes to the tests are artifacts of changing how variables need to be passed to specific methods or classes in the project.
detectcdn
For this sub-module we tested all functions by running them on working and non-working domains.
- IP resolution function
- WHOIS function
- CNAME function
- HTTPS function
- All checks function
cdnengine
For this we test if the cdnengine can properly handle sets of domains:
- Test chef object initialization
- Test grab_cdn with set of domains with definite CDNs
- Test has_cdn with domains that have CDNs and domains with no CDN
- Test that the basics of importing domains works for chef.
findcdn
We test in this module the ability for us to validate and pass information to cdnengine as well as exporting information:
- Test basic list works to pass to cdnengine
- Make sure our verbose setting is working
- Test if detection of fake or broken domains works
- Check if our input or outfiles exist or do not exist
๐ฅ Types of Changes
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (causes existing functionality to change)
โ Checklist
- [x] My code follows the code style of this project.
- [x] My change requires a change to the documentation.
- [x] I have updated the documentation accordingly.
- [x] I have read the CONTRIBUTING document.
- [x] I have added tests to cover my changes.
- [x] All new and existing tests passed.
This pull request introduces 1 alert when merging 6dce6de16c32b4f066997a518e4233e8f13e93a1 into 25eb36eed42c398995f2eb6bf55b20f9dd4008ca - view on LGTM.com
new alerts:
- 1 for Except block handles 'BaseException'
Fixing tests.
This pull request introduces 1 alert when merging a11c66708586ca15d5b6ee8b7dfcd422cce77459 into 25eb36eed42c398995f2eb6bf55b20f9dd4008ca - view on LGTM.com
new alerts:
- 1 for Except block handles 'BaseException'
This pull request introduces 1 alert when merging 38e77d52235c3a7f8bb942456bee1f3772b4a8ec into 25eb36eed42c398995f2eb6bf55b20f9dd4008ca - view on LGTM.com
new alerts:
- 1 for Except block handles 'BaseException'
@Pascal-0x90 - Thanks for fixing those two things I mentioned before, and for all the cleanup you have done here.
Ideally I'd like to see those BaseException exception blocks go away and the exec() call, but I'm not familiar enough with this code to know how to do that at the moment. Hence I'm OK with approving for now and leaving those items as future enhancements.
To make sure that this doesn't get forgotten, do you mind creating issues for those two items, mentioning the appropriate # nosec and/or #pylint comments that flag where they occur?
@Pascal-0x90 - Thanks for fixing those two things I mentioned before, and for all the cleanup you have done here.
Ideally I'd like to see those
BaseExceptionexception blocks go away and theexec()call, but I'm not familiar enough with this code to know how to do that at the moment. Hence I'm OK with approving for now and leaving those items as future enhancements.To make sure that this doesn't get forgotten, do you mind creating issues for those two items, mentioning the appropriate
# nosecand/or#pylintcomments that flag where they occur?
You're welcome @jsf9k . Below I have addresses some reasons for the ignores I put in the code (though really I am sure there is some alternative but I was not sure at the moment). I have opened some issues in regards to the parts of the code which include these ignores. Thank you again for your help and comments to improve the project's code ๐ .
Issues
- Issue #18 : Use of
exec()in setup.py - Issue #19: Use of
BaseExceptionin cdnengine.py - Issue #20: Use of
urlopen()in cdn_check.py - Issue #21: Uses of ignores in __init__.py files
This pull request introduces 1 alert when merging 968bbb69bf91753edbbd109531d2b1c1c72f87de into 25eb36eed42c398995f2eb6bf55b20f9dd4008ca - view on LGTM.com
new alerts:
- 1 for Except block handles 'BaseException'