HUE-7822 [core] Unable to clone oozie workflow in Hue 4.0

Review Request #12276 - Created Dec. 27, 2017 and submitted

Roohi Syeda
hue
HUE-7774-Autocomplete
HUE-7822
hue
enricoberti, jgauthier, johan, ranade, romain, weixia, yingc
commit 78e3a3e713ed4770bc1a7598dc731b7bc02f880d
Author: Roohi <roohisyeda@cloudera.com>
Date:   Wed Dec 27 17:05:23 2017 -0800

    HUE-7822 [core] Unable to clone oozie workflow in Hue 4.0

:100644 100644 90e425796e... 1fcfd2e416... M	desktop/core/src/desktop/api2.py
:100644 100644 f9c35d4ea3... 01088c10fd... M	desktop/core/src/desktop/static/desktop/js/apiHelper.js
:100644 100644 de60c002dc... f71a254ca2... M	desktop/core/src/desktop/static/desktop/js/document/hueFileEntry.js
:100644 100644 f4a4cf0460... e880d1acde... M	desktop/core/src/desktop/templates/document_browser.mako
:100644 100644 d7015c36bc... 7945bad9dd... M	desktop/core/src/desktop/urls.py

Manual on Chrome

  • 0
  • 0
  • 26
  • 0
  • 26
Description From Last Updated
  1. JS looks fine, someone else should take a look at the backend code.

  2. Nit: extra lines

  3. 
      
  1. Nice!

    A few points but the overall feature looks great.

    Also add a test in desktop.tests_doc2?

  2. desktop/core/src/desktop/api2.py (Diff revision 1)
     
     

    nit: drop new line

  3. desktop/core/src/desktop/api2.py (Diff revision 1)
     
     

    Actually, even a 'read' permission should allow a copy.

    Removing 'perm_type='write'' should be more correct

  4. desktop/core/src/desktop/api2.py (Diff revision 1)
     
     

    Compatibility

  5. desktop/core/src/desktop/api2.py (Diff revision 1)
     
     
     
     

    Actually should not be needed anymore

  6. desktop/core/src/desktop/api2.py (Diff revision 1)
     
     

    In case of workflow, we also need to:
    - update the json document
    - check for the workspace directory

    cf. https://github.com/cloudera/hue/blob/master/apps/oozie/src/oozie/views/editor2.py#L182

  7. Would make more sense to combine with the other operations?

    (btw: moving 'Rename folder' below 'Move to trash' at the same time)

    Could be on position 1

  8. 
      
  1. 
      
  2. desktop/core/src/desktop/tests_doc2.py (Diff revision 2)
     
     

    I am getting an error here. At line https://github.com/cloudera/hue/blob/004deabdee4527910c2d80ee4df208ef783d0790/desktop/libs/hadoop/src/hadoop/pseudo_hdfs4.py#L332

      File "/Users/fnusyedaroohi/github/cloudera/hue/desktop/core/src/desktop/tests_doc2.py", line 48, in setUp
        self.cluster = pseudo_hdfs4.shared_cluster()
      File "/Users/fnusyedaroohi/github/cloudera/hue/desktop/libs/hadoop/src/hadoop/pseudo_hdfs4.py", line 556, in shared_cluster
        cluster.start()
      File "/Users/fnusyedaroohi/github/cloudera/hue/desktop/libs/hadoop/src/hadoop/pseudo_hdfs4.py", line 256, in start
        self._format(self.hadoop_conf_dir, env)
      File "/Users/fnusyedaroohi/github/cloudera/hue/desktop/libs/hadoop/src/hadoop/pseudo_hdfs4.py", line 332, in _format
        ret = subprocess.call(args, env=env, stdout=stdout, stderr=stderr)
      File "/usr/local/Cellar/python/2.7.14_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 168, in call
        return Popen(*popenargs, **kwargs).wait()
      File "/usr/local/Cellar/python/2.7.14_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 390, in __init__
        errread, errwrite)
      File "/usr/local/Cellar/python/2.7.14_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 1025, in _execute_child
        raise child_exception
    OSError: [Errno 2] No such file or directory
    
  3. 
      
  1. 
      
  2. desktop/core/src/desktop/tests_doc2.py (Diff revision 2)
     
     

    This is because we don't want to point to a real cluster with HDFS as it does not exist.

    Instead could you mock the fs?

    e.g.
    https://github.com/cloudera/hue/blob/master/desktop/libs/liboozie/src/liboozie/submittion2_tests.py#L165

  3. 
      
  1. 
      
  2. desktop/core/src/desktop/api2.py (Diff revision 3)
     
     

    Won't doing the import here of an app in desktop break 'make apps' or starting Hue? (cyclic dependency)

    Might need to move to #382

  3. desktop/core/src/desktop/api2.py (Diff revision 3)
     
     

    .get('uuid', '""')

    or would get a json error

  4. desktop/core/src/desktop/api2.py (Diff revision 3)
     
     

    document2 --> document?

  5. desktop/core/src/desktop/api2.py (Diff revision 3)
     
     

    copydocument2 --> copied_document?

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

    nit: to move up with other desktop imports

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

    The problem here and below is that we don't revert the changes, so it will brake the other tests of Hue.

    Could see if we have other examples of Monkey patching in other tests or do in a

    try/finally where we make sure in the finally to revert to the real check_workspace

  8. desktop/core/src/desktop/tests_doc2.py (Diff revision 3)
     
     

    Same, we would need to revert to use the real ProxyFS.copy_remote_dir in the finally

  9. desktop/core/src/desktop/tests_doc2.py (Diff revision 3)
     
     

    space after , and not after =

  10. 
      
  1. Almost, nice!

  2. desktop/core/src/desktop/api2.py (Diff revision 4)
     
     

    For the other types of documents (as we now allow any doc to be copied), shouldn't we look if there is a 'uuid' in the data dict of the document and update the uuids there with the new one too? (that way it matches the new uuid in the copied document2)

    1. I realised that there are several types to support. I am not sure my code covers all.

  3. desktop/core/src/desktop/tests_doc2.py (Diff revision 4)
     
     
     

    Does this work? (we seem to have 3 args amd call with 2 only)

    1. Yes, I think the other arg is self

  4. desktop/core/src/desktop/tests_doc2.py (Diff revision 4)
     
     

    Isn't check_workspace called by _import_workspace which will be called on ' response = self.client.post('/desktop/api2/doc/copy'?

    (so below)

  5. desktop/core/src/desktop/tests_doc2.py (Diff revision 4)
     
     

    also assert both uuids are different?
    (at the json + document level)

  6. 
      
  1. 
      
  2. desktop/core/src/desktop/api2.py (Diff revision 4)
     
     

    Thanks for checking Romain. As I was looking at the old code. I also see that I need to create workspace for oozie bundle & coordinator too https://github.com/cloudera/hue/blob/22f8466894b4453c5f7f749542b5f777c1126468/apps/oozie/src/oozie/views/editor2.py#L558
    Another thing I observed is that only the name and the UUID is not being updated in the old code. Am I missing something? I tested my new code for Workflow and see that UUID is not being updated too.

    1. About the UUID, it is probably because it is not used directly from the json.

      but it is there in the ko model: https://github.com/cloudera/hue/blob/master/apps/oozie/src/oozie/static/oozie/js/workflow-editor.ko.js#L191

      so just add when loading the workflow with a document
      https://github.com/cloudera/hue/blob/master/apps/oozie/src/oozie/models2.py#L417
      _data['workflow']['uuid'] = self.document.uuid

      ?

  3. 
      
  1. 
      
  2. desktop/core/src/desktop/api2.py (Diff revision 5)
     
     

    I think we need extra code to support Directory, right now, it only copies directory not it contents.

  3. 
      
  1. Nice, last nits!

  2. desktop/core/src/desktop/api2.py (Diff revision 5)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    +1 to merge both together?

    i.e.

    doing
    json_data['name'] = name
    json_data['uuid'] = copy_document.uuid

    for the workflow too

    1. I tried this several times, but it didn't work, that is why I added a new method update_uuid for workflow. I am guessing it might be because, we have don't have nested dictionary in bundle or coordinator, but for workflow we need to update 'workflow' json only.

  3. desktop/core/src/desktop/api2.py (Diff revision 5)
     
     
     

    +1 to disable directory copy here and UI

  4. desktop/core/src/desktop/tests_doc2.py (Diff revision 5)
     
     
     
     

    Why do we need that as it should be called under the cover in ' response = self.client.post('/desktop/api2/doc/copy', {'

    ?

    1. This is the original workflow which we are copying. I followed these steps for creating a new workflow https://github.com/cloudera/hue/blob/22f8466894b4453c5f7f749542b5f777c1126468/apps/oozie/src/oozie/views/editor2.py#L126

  5. 
      
  1. Ship It!
  2. 
      
Review request changed

Status: Closed (submitted)

Loading...