Change in vdsm[master]: ssl: runtime config to choose implementation

fromani at redhat.com fromani at redhat.com
Thu Sep 10 15:51:06 UTC 2015


Francesco Romani has posted comments on this change.

Change subject: ssl: runtime config to choose implementation
......................................................................


Patch Set 6: Code-Review-1

(5 comments)

-1 for visibility
minor documentation comments, one suggestion

https://gerrit.ovirt.org/#/c/44689/6/lib/vdsm/config.py.in
File lib/vdsm/config.py.in:

Line 207: 
Line 208:         ('connection_stats_timeout', '3600',
Line 209:             'Time in seconds defining how frequently we log transport stats'),
Line 210: 
Line 211:         ('ssl_impl', 'm2c',
oh dear, an old comment of mine got lost...
Let me repaste:
no need to be so verbose here, let's change this to 'ssl_implementation' (or whaterver else you feel more appropriate, just be verbose)
Line 212:             'Specifies which ssl impl should be used. There are 2 options: '
Line 213:             'm2crypto (by default) defined as m2c and python standard ssl '
Line 214:             'module defined as ssl'),
Line 215:     ]),


Line 208:         ('connection_stats_timeout', '3600',
Line 209:             'Time in seconds defining how frequently we log transport stats'),
Line 210: 
Line 211:         ('ssl_impl', 'm2c',
Line 212:             'Specifies which ssl impl should be used. There are 2 options: '
same, we can spare 'ssl implementation', no need to use abbreviation s in documentation.
Line 213:             'm2crypto (by default) defined as m2c and python standard ssl '
Line 214:             'module defined as ssl'),
Line 215:     ]),
Line 216: 


Line 210: 
Line 211:         ('ssl_impl', 'm2c',
Line 212:             'Specifies which ssl impl should be used. There are 2 options: '
Line 213:             'm2crypto (by default) defined as m2c and python standard ssl '
Line 214:             'module defined as ssl'),
I'd write:

  'There are two options: '
  '"m2c" to use the m2crypto module (default) '
  '"ssl" to use the standard python ssl module) '

I'm not a native english speaker, so feel free to add further corrections.
Line 215:     ]),
Line 216: 
Line 217:     # Section: [mom]
Line 218:     ('mom', [


https://gerrit.ovirt.org/#/c/44689/6/lib/vdsm/sslcompat.py
File lib/vdsm/sslcompat.py:

Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20: from .config import config
Line 21: try:
Line 22:     if config.get('vars', 'ssl_impl') == 'm2c':
ssl_implementation - or whatever long name you want to use
Line 23:         from . import m2cutils as sslutils
Line 24:         from .m2cutils import SSLSocket
Line 25:         from .m2cutils import SSLHandshakeDispatcher
Line 26:     else:


Line 26:     else:
Line 27:         from . import sslutils
Line 28:         from .sslutils import SSLHandshakeDispatcher
Line 29:         from ssl import SSLSocket
Line 30: except ImportError:
> maybe use ssl=False before importing, and set it in case you need import.
Maybe something like

  from .config import config
  
  USED_IMPLEMENTATION = None
  
  if config.get('vars', 'ssl_impl') == 'm2c':
       try:
          from . import m2cutils as sslutils
          from .m2cutils import SSLSocket
          from .m2cutils import SSLHandshakeDispatcher
       except ImportError:
           # m2crpyto was request but it is unavailable.
           # worth a log line?
           pass  
       else:
           USED_IMPLEMENTATION = 'm2c'
  
  if USED_IMPLEMENTATION is None:
      from . import sslutils
      from .sslutils import SSLHandshakeDispatcher
      from ssl import SSLSocket
      USED_IMPLEMENTATION = 'ssl'
  
  
  # either way, we  need it to satisfy pyflakes
  sslutils
  SSLHandshakeDispatcher
  SSLSocket
Line 31:     from . import sslutils
Line 32:     from .sslutils import SSLHandshakeDispatcher
Line 33:     from ssl import SSLSocket
Line 34: 


-- 
To view, visit https://gerrit.ovirt.org/44689
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9881d11e30ced9c34bfe602bba3d968f57e0fe15
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo at redhat.com>
Gerrit-Reviewer: Simone Tiraboschi <stirabos at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list