#19 tox fixes
Merged by ralph. Opened by fivaldi.
fivaldi/freshmaker fivaldi_ansible_tox  into  master

Download 19.patch
no initial comment

rebased

This PR is ready for review.

Running this with detox works for me.

bandit and flake8 both found lots of "problems".

Someone else should probably review this too though (one of @jkaluza, @cqi, or @qwan).

Works for me by running tox, but got warnings

coverage runtests: commands[1] | coverage combine
WARNING:test command found but not installed in testenv
  cmd: /usr/bin/coverage
  env: /home/cqi/code/factory2/code/freshmaker/.tox/coverage
Maybe you forgot to specify a dependency? See also the whitelist_externals envconfig setting.
coverage runtests: commands[2] | coverage report --omit=tests/*,.tox/*,/usr/* -m --skip-covered
WARNING:test command found but not installed in testenv
  cmd: /usr/bin/coverage
  env: /home/cqi/code/factory2/code/freshmaker/.tox/coverage
Maybe you forgot to specify a dependency? See also the whitelist_externals envconfig setting.

Relative to this change?

Suggest to merge this patch and use another PR to fix those "problems".

There are some flake8 errors I've been fixed in #32, for some other flake8 errors reported against migrations/*, we can ignore them since they're generated automatically.

For bandit reported errors, all are expected behavior can be ignored:
1. bind to 0.0.0.0 interface in config
2. Try-Except-Pass in config to override config_section
3. using subprocess risk
4. harded coded '/tmp' as default value of rundir in utils._run_command

For coverage warning, it's caused by sitepackages = True, but we need it due to 'koji'... so can ignore it too.

we can ignore files under freshmaker/migrations by excluding them here

Try-Except-Pass in config to override config_section

Replace with ?

if os.path.exists(config_file) and os.access(config_file, os.R_OK):
    config_section = 'ProdConfiguration'

harded coded '/tmp' as default value of rundir in utils._run_command

/tmp could be replaced with tempfile.gettempdir()

Try-Except-Pass in config to override config_section

Replace with ?
if os.path.exists(config_file) and os.access(config_file, os.R_OK):
config_section = 'ProdConfiguration'

It is desirable to suppress exceptions here, so I don't think we need the change.

harded coded '/tmp' as default value of rundir in utils._run_command

/tmp could be replaced with tempfile.gettempdir()

Bandit reports this potential security issue, but actually not true in this case, I think we can ignore it too. Reference: https://security.openstack.org/guidelines/dg_using-temporary-files-securely.html

We can get the system temporary directory from Python standard library, why do we need to hard-coded it?

We can get the system temporary directory from Python standard library, why do we need to hard-coded it?

Anyway, it doesn't hurt to change, so I don't have strong option against doing this, added a new commit for this in #32.

Suggest to merge this patch and use another PR to fix those "problems".

Agreed. Let's merge this and #32, and iterate from there.

Pull-Request has been merged by ralph

Metadata