[x509] Add Swapper #86 #87
Conversation
There was a problem hiding this comment.
Great progress.
Can you add some docs regarding the swapper feature?
Regarding the rest, I'd use this structure for the tests:
tests/
tests/openwisp2/ (project name)
tests/openwisp2/settings.py (project settings)
tests/openwisp2/sample_x509/ ... (files of the sample app here)
The sample_x509 directory should contain the logic which is executed only when SAMPLE_APP=1.
The rest goes /tests/openwisp2.
Docs for extending django-x509 already exist. Other than this, |
atb00ker
force-pushed
the
swapper
branch
4 times, most recently
from
437a82a
to
2694522
Compare
atb00ker
force-pushed
the
swapper
branch
4 times, most recently
from
f6d0c5d
to
4d5c8af
Compare
|
Rebased with master! 😄 |
There was a problem hiding this comment.
Great work. It's almost ready to merge, I left some comments below regarding minor improvements.
As part of the improvement we're making, can you add some screenshots (and/or animated gifs if you fancy this) in the initial part of the README like we have on some other modules? As part of the merge process I would like to standardize this and ensure we have nice images that help us to promote the modules.
For example:
- importing a CA cert
- creating a CA or cert
- showing page where details of the certificates are presented
- renewing a CA
I think we have to add the black reformatting in a separate PR as we did in other repos, but we can do after merging this PR, I created the issue: #90
PS: could you rebase on the latest master as well please?
atb00ker
force-pushed
the
swapper
branch
9 times, most recently
from
503be3c
to
5913c95
Compare
django_x509/tests/base.py
Outdated
| ca.save() | ||
| return ca | ||
|
|
||
| def _create_cert(self, cert_model=None, **kwargs): |
atb00ker
force-pushed
the
swapper
branch
8 times, most recently
from
c3ba0d9
to
38a5f19
Compare
atb00ker
force-pushed
the
swapper
branch
3 times, most recently
from
e71cd61
to
6c8515d
Compare
There was a problem hiding this comment.
I did one last test in openwisp-controller, I tried pulling these changes and use it with the current openwisp-controller, but an error comes up:
ImportError: cannot import name 'TestX509Mixin' from 'django_x509.tests' (/home/nemesis/Code/openwisp/django-x509/django_x509/tests/__init__.py)There may be others.
Can you patch these changes to make them backward compatible please?
You can test using the current openwisp-controller version.
See my comments below as well please.
atb00ker
force-pushed
the
swapper
branch
2 times, most recently
from
be58a08
to
9b80242
Compare
|
@nemesisdesign Now, you only need to add: To test it in the current openwisp-controller version. |
requirements.txt
Outdated
| @@ -3,3 +3,4 @@ django-model-utils>=4.0 | |||
| jsonfield>=3.1.0,<4.0.0 | |||
| cryptography>=2.4.0,<2.10.0 | |||
| pyopenssl>=17.5.0,<20.0.0 | |||
| swapper~=1.1.0 | |||
There was a problem hiding this comment.
I think this should be swapper~=1.1
Pay attention: openwisp/openwisp-ipam@af5ad60
Let's make sure all dependencies are pinned
There was a problem hiding this comment.
swapper~=1.1.0 == swapper>=1.1,swapper<1.2
While,
swapper~=1.1 == swapper>=1.1,swapper<2
Wouldn't having upper limit as swapper<2 be very lenient given swapper updates are infrequent and not documented?
There was a problem hiding this comment.
Thanks for clarifying, I think we can relax the requirement because this module is pretty stable.
Commands used to convert mkv to gif in the README
ffmpeg -i vid.mkv -vf fps=7,scale=1366:-1:flags=lanczos,palettegen palette.png ffmpeg -i vid.mkv -i palette.png -filter_complex "fps=7,scale=1366:-1:flags=lanczos[x];[x][1:v]paletteuse" sixthtry.gifCloses #86