cnaas-nms
cnaas-nms copied to clipboard
Add support for branch refspecs in Git repo urls
This will parse any fragment present in a Git repo url as a branch refspec, and make sure to clone that branch when the repo is first cloned.
I would have liked to add some unit tests for the new function, but I am unable to run said unit tests in isolation, because the existing code base carries the sad code smell of simple module imports causing side-effects: Just attempting to run the tests present in cnaas_nms.db.tests.test_git
ultimately causes the cnaas_nms.db.session
module to be imported, and this module tries to connect to a running database at import time. This problem should be fixed separately.
Fixes #222
Codecov Report
Merging #223 (8fe1db1) into develop (52840ef) will decrease coverage by
4.36%
. The diff coverage is100.00%
.
:exclamation: Current head 8fe1db1 differs from pull request most recent head b5d5b0f. Consider uploading reports for the commit b5d5b0f to get more accurate results
@@ Coverage Diff @@
## develop #223 +/- ##
===========================================
- Coverage 63.78% 59.42% -4.37%
===========================================
Files 63 61 -2
Lines 6627 6186 -441
===========================================
- Hits 4227 3676 -551
- Misses 2400 2510 +110
Impacted Files | Coverage Δ | |
---|---|---|
src/cnaas_nms/db/git.py | 55.04% <100.00%> (-8.24%) |
:arrow_down: |
src/cnaas_nms/api/linknet.py | 19.56% <0.00%> (-46.93%) |
:arrow_down: |
src/cnaas_nms/confpush/underlay.py | 19.60% <0.00%> (-37.26%) |
:arrow_down: |
src/cnaas_nms/tools/jinja_filters.py | 14.70% <0.00%> (-22.03%) |
:arrow_down: |
src/cnaas_nms/db/session.py | 75.00% <0.00%> (-9.85%) |
:arrow_down: |
src/cnaas_nms/db/linknet.py | 79.71% <0.00%> (-9.32%) |
:arrow_down: |
src/cnaas_nms/api/app.py | 68.42% <0.00%> (-8.51%) |
:arrow_down: |
src/cnaas_nms/confpush/init_device.py | 50.11% <0.00%> (-7.88%) |
:arrow_down: |
src/cnaas_nms/api/repository.py | 54.76% <0.00%> (-7.15%) |
:arrow_down: |
... and 33 more |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
Needs some documentation to describe the URL format, but looks simple and clean!
Needs some documentation to describe the URL format, but looks simple and clean!
The first step to documenting this simple function is adding tests for it. Things have happened since I last visited this PR. However, on the current develop
branch, unit testing without weird dependencies still proves to be problematic:
To run just a simple test method on the introduced parse_repo_url
function, seems to require that I place a JWT certificate in /opt/cnaas/jwtcert/public.pem
, which is just ridiculous:
[2022-09-14 15:57:51,457] DEBUG in settings: Loaded settings_fields module from bundled cnaas-nms
[2022-09-14 15:57:51,514] ERROR in pki: Specified cafile is not a file: /opt/cnaas/cacert/rootCA.crt
[2022-09-14 15:57:51,515] WARNING in pki: Accepting unverified TLS certificates
[2022-09-14 15:57:52,555] DEBUG in settings: Loaded settings_fields module from bundled cnaas-nms
E
======================================================================
ERROR: test suite for <module 'cnaas_nms.db.tests.test_git' from '/Users/mvold/d/cnaas-nms/src/cnaas_nms/db/tests/test_git.py'>
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/mvold/d/cnaas-nms/.ve/lib/python3.7/site-packages/nose/suite.py", line 210, in run
self.setUp()
File "/Users/mvold/d/cnaas-nms/.ve/lib/python3.7/site-packages/nose/suite.py", line 293, in setUp
self.setupContext(ancestor)
File "/Users/mvold/d/cnaas-nms/.ve/lib/python3.7/site-packages/nose/suite.py", line 316, in setupContext
try_run(context, names)
File "/Users/mvold/d/cnaas-nms/.ve/lib/python3.7/site-packages/nose/util.py", line 471, in try_run
return func()
File "/Users/mvold/d/cnaas-nms/src/cnaas_nms/__init__.py", line 5, in setup_package
import cnaas_nms.api.app
File "/Users/mvold/d/cnaas-nms/src/cnaas_nms/api/app.py", line 90, in <module>
sys.exit(1)
SystemExit: 1
-------------------- >> begin captured stdout << ---------------------
Error in add_event: Error 61 connecting to 127.0.0.1:6379. Connection refused.
Error in add_event: Error 61 connecting to 127.0.0.1:6379. Connection refused.
Error in add_event: Error 61 connecting to 127.0.0.1:6379. Connection refused.
Error in add_event: Error 61 connecting to 127.0.0.1:6379. Connection refused.
Could not load public JWT cert from api.yml config: [Errno 2] No such file or directory: '/opt/cnaas/jwtcert/public.pem'
--------------------- >> end captured stdout << ----------------------
-------------------- >> begin captured logging << --------------------
git.cmd: DEBUG: Popen(['git', 'version'], cwd=/Users/mvold/d/cnaas-nms/src/cnaas_nms/db/tests, universal_newlines=False, shell=None, istream=None)
git.cmd: DEBUG: Popen(['git', 'version'], cwd=/Users/mvold/d/cnaas-nms/src/cnaas_nms/db/tests, universal_newlines=False, shell=None, istream=None)
passlib.utils.compat: DEBUG: loaded lazy attr 'SafeConfigParser': <class 'configparser.ConfigParser'>
passlib.utils.compat: DEBUG: loaded lazy attr 'NativeStringIO': <class '_io.StringIO'>
passlib.utils.compat: DEBUG: loaded lazy attr 'BytesIO': <class '_io.BytesIO'>
cnaas-nms: DEBUG: Loaded settings_fields module from bundled cnaas-nms
cnaas-nms: ERROR: Specified cafile is not a file: /opt/cnaas/cacert/rootCA.crt
cnaas-nms: WARNING: Accepting unverified TLS certificates
cnaas-nms: DEBUG: Loaded settings_fields module from bundled cnaas-nms
--------------------- >> end captured logging << ---------------------
----------------------------------------------------------------------
Ran 0 tests in 1.069s
FAILED (errors=1)
error in setup context
Traceback (most recent call last):
File "/Users/mvold/d/cnaas-nms/.ve/lib/python3.7/site-packages/nose/suite.py", line 210, in run
self.setUp()
File "/Users/mvold/d/cnaas-nms/.ve/lib/python3.7/site-packages/nose/suite.py", line 293, in setUp
self.setupContext(ancestor)
File "/Users/mvold/d/cnaas-nms/.ve/lib/python3.7/site-packages/nose/suite.py", line 316, in setupContext
try_run(context, names)
File "/Users/mvold/d/cnaas-nms/.ve/lib/python3.7/site-packages/nose/util.py", line 471, in try_run
return func()
File "/Users/mvold/d/cnaas-nms/src/cnaas_nms/__init__.py", line 5, in setup_package
import cnaas_nms.api.app
File "/Users/mvold/d/cnaas-nms/src/cnaas_nms/api/app.py", line 90, in <module>
sys.exit(1)
Exception: 1
-------------------- >> begin captured logging << --------------------
git.cmd: DEBUG: Popen(['git', 'version'], cwd=/Users/mvold/d/cnaas-nms/src/cnaas_nms/db/tests, universal_newlines=False, shell=None, istream=None)
git.cmd: DEBUG: Popen(['git', 'version'], cwd=/Users/mvold/d/cnaas-nms/src/cnaas_nms/db/tests, universal_newlines=False, shell=None, istream=None)
passlib.utils.compat: DEBUG: loaded lazy attr 'SafeConfigParser': <class 'configparser.ConfigParser'>
passlib.utils.compat: DEBUG: loaded lazy attr 'NativeStringIO': <class '_io.StringIO'>
passlib.utils.compat: DEBUG: loaded lazy attr 'BytesIO': <class '_io.BytesIO'>
cnaas-nms: DEBUG: Loaded settings_fields module from bundled cnaas-nms
cnaas-nms: ERROR: Specified cafile is not a file: /opt/cnaas/cacert/rootCA.crt
cnaas-nms: WARNING: Accepting unverified TLS certificates
cnaas-nms: DEBUG: Loaded settings_fields module from bundled cnaas-nms
--------------------- >> end captured logging << ---------------------
Incidentally, I also find this to be kind of silly. What is the point of the sys.exit(1)
in this case? Also produces a pretty bad side-effect during testing...:
https://github.com/SUNET/cnaas-nms/blob/9e08e7fa5e02deeb7c77b443b9f27976a5eb2f46/src/cnaas_nms/api/app.py#L86-L90
Ideally, the test suite should be able to set up a test environment where any necessary configuration is present, and not be dependent on files being present in specific absolute paths. Also, ideally, no such config should be necessary just to test the parse_repo_url()
function. It mostly boils down (again) to code that is setting up application state as a side-effect of simply importing a Python module.
You know the codebase better than me, @indy-independence; what do you think would be the best strategy to mitigate this? I really want to get on with adding value to CNaaS-NMS, but the simple things are made so hard...
Needs some documentation to describe the URL format, but looks simple and clean!
Anyhoo, I added some suggested docs. Since there are multiple GITREPO settings where this can be used, I suggested adding a new section to the config docs.