HUE-7675 [core] Warn on startup for any invalid configurations in hue.ini

Review Request #12303 - Created Jan. 5, 2018 and submitted

Roohi Syeda
hue
HUE-7774-Autocomplete
HUE-7675
hue
enricoberti, jgauthier, johan, ranade, romain, weixia, yingc
commit f631905b9a56404be2555689c4eb01de1edeac93
Author: Roohi <roohisyeda@cloudera.com>
Date:   Fri Jan 5 13:32:04 2018 -0800

    HUE-7675 [core] Warn on startup for any invalid configurations in hue.ini (Step#1)

:100644 100644 fd61bcd200... c1f6e6df86... R091	desktop/core/ext-py/configobj/configobj.py	desktop/core/ext-py/configobj-4.7.2/configobj.py
:000000 100644 0000000000... 9023a7e014... A	desktop/core/ext-py/configobj-4.7.2/docs/configobj.html
:000000 100644 0000000000... 7dab99cc6d... A	desktop/core/ext-py/configobj-4.7.2/docs/configobj.txt
:100644 100644 8ae904d0a9... 030cf1719a... R080	desktop/core/ext-py/configobj/docs/stylesheets/default.css	desktop/core/ext-py/configobj-4.7.2/docs/default.css
:000000 100644 0000000000... 0992ff26d2... A	desktop/core/ext-py/configobj-4.7.2/docs/docutils.conf
:100644 100644 4a626f1a5e... 81b3e76075... R098	desktop/core/ext-py/configobj/docs/stylesheets/voidspace_docutils.css	desktop/core/ext-py/configobj-4.7.2/docs/docutils.css
:000000 100644 0000000000... 122b4294b9... A	desktop/core/ext-py/configobj-4.7.2/docs/pygments.css
:000000 100644 0000000000... 2591bce35d... A	desktop/core/ext-py/configobj-4.7.2/docs/template.tmp
:000000 100644 0000000000... f514475752... A	desktop/core/ext-py/configobj-4.7.2/docs/validate.html
:000000 100644 0000000000... acdb263194... A	desktop/core/ext-py/configobj-4.7.2/docs/validate.txt
:000000 100644 0000000000... 63d70cc0c8... A	desktop/core/ext-py/configobj-4.7.2/setup.py
:000000 100644 0000000000... e69de29bb2... A	desktop/core/ext-py/configobj-4.7.2/tests/functionaltests/__init__.py
:000000 100644 0000000000... aa429fff49... A	desktop/core/ext-py/configobj-4.7.2/tests/functionaltests/conf.ini
:000000 100644 0000000000... 3af70acf83... A	desktop/core/ext-py/configobj-4.7.2/tests/functionaltests/conf.spec
:000000 100644 0000000000... 8afa37f03b... A	desktop/core/ext-py/configobj-4.7.2/tests/functionaltests/test_configobj.py
:000000 100644 0000000000... 4517817d15... A	desktop/core/ext-py/configobj-4.7.2/tests/functionaltests/test_validate_errors.py
:000000 100644 0000000000... d4a3410237... A	desktop/core/ext-py/configobj-4.7.2/tests/test_configobj.py
:100644 100644 30bdfacbe8... 73dbdb891b... R098	desktop/core/ext-py/configobj/validate.py	desktop/core/ext-py/configobj-4.7.2/validate.py
:100644 000000 744c153f86... 0000000000... D	desktop/core/ext-py/configobj/PKG-INFO
:100644 000000 3da3a8d3f8... 0000000000... D	desktop/core/ext-py/configobj/docs/configobj.html
:100644 000000 2e9d99c2d7... 0000000000... D	desktop/core/ext-py/configobj/docs/images/PythonPowered.png
:100644 000000 aee56bec20... 0000000000... D	desktop/core/ext-py/configobj/docs/images/new_python.gif
:100644 000000 91cd58f670... 0000000000... D	desktop/core/ext-py/configobj/docs/images/osi-certified-120x100.gif
:100644 000000 f1465f98ac... 0000000000... D	desktop/core/ext-py/configobj/docs/images/powered_by_python.jpg
:100644 000000 5b559c63e8... 0000000000... D	desktop/core/ext-py/configobj/docs/images/pythonbanner.gif
:100644 000000 ea3b477d2d... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/arrow.gif
:100644 000000 7c4bc3e466... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/badgrin.gif
:100644 000000 b64b561f00... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/biggrin.gif
:100644 000000 b4cca3bba3... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/confused.gif
:100644 000000 813f9e4cbd... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/cool.gif
:100644 000000 747f764db7... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/cry.gif
:100644 000000 1e3c2dceb2... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/doubt.gif
:100644 000000 4f88dae2a5... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/evil.gif
:100644 000000 46f576d6fa... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/exclaim.gif
:100644 000000 8e9a67329d... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/idea.gif
:100644 000000 11618c5941... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/lol.gif
:100644 000000 808e079c68... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/mad.gif
:100644 000000 ffc87920c0... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/neutral.gif
:100644 000000 a1d0230078... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/question.gif
:100644 000000 b83b6c6768... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/razz.gif
:100644 000000 4fdd3914bd... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/redface.gif
:100644 000000 8ec17a0007... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/rolleyes.gif
:100644 000000 60da9d9741... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/sad.gif
:100644 000000 7e3cd6fd9a... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/sc_smilies.pak
:100644 000000 c3c40b5352... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/shock.gif
:100644 000000 55ae95b1f2... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/smile.gif
:100644 000000 336a7852b0... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/surprised.gif
:100644 000000 dc09e0a52d... 0000000000... D	desktop/core/ext-py/configobj/docs/smilies/wink.gif
:100644 000000 f98e7a5f04... 0000000000... D	desktop/core/ext-py/configobj/docs/stylesheets/pep.css
:100644 000000 48d4095ce7... 0000000000... D	desktop/core/ext-py/configobj/docs/stylesheets/pysrc.css
:100644 000000 1c990f2bda... 0000000000... D	desktop/core/ext-py/configobj/docs/validate.html
:100644 000000 722f99ae3d... 0000000000... D	desktop/core/ext-py/configobj/setup.py
:000000 100644 0000000000... 10dbc36c5b... A	desktop/core/src/desktop/management/commands/configspec_dump.py

https://master-02.jenkins.cloudera.com/user/ranade/my-views/view/Hue-Builds/job/Hue-Build-Master/219/
Manually on chrome

  • 0
  • 0
  • 31
  • 14
  • 45
Description From Last Updated
  1. Was there a lib upgrade or not? Review commit message does not match.
    What was the testing?

    It is nice to update the default values that are incorrect in the inis but not to remove existing configurations

  2. Could we leave empty?

  3. Could we keep commented?

    1. Sorry this file was included by mistake in the commit and removed it and updated the review too.

  4. Could we keep if not dev Hue won't have it

  5. desktop/conf/pseudo-distributed.ini.tmpl (Diff revision 2)
     
     
     
     
     
     
     
     
     

    Could we comment? If the default values are not there, could you set them in the conf.py?

    (we should not need to uncomment non dev stuff in this template)

  6. Seems invalid new ”

  7. A lot of stuff seems gone below?

  8. desktop/core/src/desktop/lib/conf.py (Diff revision 2)
     
     

    Why can't we generate most of this file on the fly?
    (and tweak it afterwards if needed)

    1. The generated Config spec is based on reading an .ini file (which is supposed to be correct) and based on the config spec we have to actually validate the (other) .ini file. I am not sure how to separate them out.

  9. 
      
  1. 
      
  2. desktop/conf/hue.ini.spec (Diff revision 4)
     
     

    This is the spec file generated for validating the hue.ini file later

  3. desktop/conf/hue.ini.spec (Diff revision 4)
     
     
    This is what I was talking about. Actually the validation fails because the spec file is itself incorrect as it has duplicate [[[__many__]]]. Please compare this to [zookeeper] [[cluster]] [[__many__]].
    Right now I am manually deleting it. If there are repeating anonymous sections, configspec validation needs it to be defined as __many__. 
    http://configobj.readthedocs.io/en/latest/configobj.html#repeated-sections
  4. desktop/core/src/desktop/lib/conf.py (Diff revision 4)
     
     

    I will remove print after testing.

  5. This is the main file which generated the spec file. This should be run everytime some changes are made to hue.ini file (adding/removing sections/values). I am not sure if this is the right approach

  6. There is an issue with the spec validation module. It expects kv's to appear before any sections

  7. 
      
  1. Nice!

    How about refactoring so that we can unit test it easily? (with some corner cases)

  2. Move file to desktop/lib?

  3. space after around *

  4. if self.level != 0:

  5. same for * and spaces

  6. desktop/core/src/desktop/lib/conf.py (Diff revision 5)
     
     

    space after ,

  7. desktop/core/src/desktop/views.py (Diff revision 5)
     
     

    Could you move it down with the other dsktp imports?

  8. desktop/core/src/desktop/views.py (Diff revision 5)
     
     

    Could you move it up with Django imports?

  9. desktop/core/src/desktop/views.py (Diff revision 5)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    BTW: should we move this in a function so that we can add a test?

  10. desktop/core/src/desktop/views.py (Diff revision 5)
     
     

    Maybe generate as a tmp file instead?

  11. desktop/core/src/desktop/views.py (Diff revision 5)
     
     

    why not 'validator' and not 'vtor'?

  12. desktop/core/src/desktop/views.py (Diff revision 5)
     
     
    nit: sometimes we do 
    
    message = []
    
    message.append(string ...)
    
    
    '\n'.join(message)
    
    
    (note: \n creates a new line in the warning, or we should just not put it?)
  13. desktop/core/src/desktop/views.py (Diff revision 5)
     
     

    nit: new line above and not below

  14. desktop/core/src/desktop/views.py (Diff revision 5)
     
     

    LOG.exception() or warn ?

  15. desktop/core/src/desktop/views.py (Diff revision 5)
     
     

    if _CONFIG_ERROR_LIST and _CONFIG_ERROR_LIST:

    ?

    of even just

    if _CONFIG_ERROR_LIST:

  16. 
      
  1. Very nice!

    Bunch of little nits but the core is great

  2. beware, better use another name than 'list' which overrides the list type and is also not self descriptive

  3. desktop/core/src/desktop/tests.py (Diff revision 6)
     
     

    Also add another test where it autovalidates itself against the default hue.ini?

  4. desktop/core/src/desktop/tests.py (Diff revision 6)
     
     

    Could we add an expected key and sub section? (to test against basic false positive)

  5. desktop/core/src/desktop/views.py (Diff revision 6)
     
     

    use tmp file instead of?

    os.path.join(get_desktop_root("conf"), "hue.ini.tmp")

    e.g.

    https://docs.python.org/2/library/tempfile.html#tempfile.mkstemp
    tempfile.mkdtemp(prefix='tmp_hue_', dir=TEST_HDFS_TMP_DIR)

  6. desktop/core/src/desktop/views.py (Diff revision 6)
     
     

    , % (name, str(ex))

  7. desktop/core/src/desktop/views.py (Diff revision 6)
     
     

    , % (name, str(ex))

  8. desktop/core/src/desktop/views.py (Diff revision 6)
     
     

    'Extra %s '%s' in the [%s].'

    [] might no be correct but should be more intuitive to the user where to look at

    BTW
    Don't we usually internationalize the string?

  9. desktop/core/src/desktop/views.py (Diff revision 6)
     
     

    if message:

  10. desktop/core/src/desktop/views.py (Diff revision 6)
     
     

    ', '.join ?

  11. 
      
  1. 
      
  2. desktop/core/src/desktop/lib/conf.py (Diff revision 7)
     
     

    get_extra_values used?

  3. desktop/core/src/desktop/tests.py (Diff revision 7)
     
     
     
     
     
     
     

    try:
    ..
    finally:
    os.remove(..)

    to delete it for sure

  4. desktop/core/src/desktop/tests.py (Diff revision 7)
     
     

    assert_equal(len(error_list), 0)

    or

    assert_false(error_list)

  5. desktop/core/src/desktop/tests.py (Diff revision 7)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
  6. desktop/core/src/desktop/tests.py (Diff revision 7)
     
     

    assert_equal?

    and

    assert(len(error_list), 1)

    ?

  7. desktop/core/src/desktop/views.py (Diff revision 7)
     
     
     
     
     
     
  8. desktop/core/src/desktop/views.py (Diff revision 7)
     
     
     
     
     
  9. 
      
Review request changed

Status: Closed (submitted)

Loading...