[GitHub] [incubator-heron] Code0x58 edited a comment on pull request #3588: Deep clean heron-explorer

[GitBox <gi...@apache.org>]

Code0x58 edited a comment on pull request #3588:
URL: https://github.com/apache/incubator-heron/pull/3588#issuecomment-667589347


   Thanks for checking it out @huijunw!
   
   1. Updated to default to WARNING and above, then can take -v/--verbose for INFO, and another -v/--verbose for DEBUG
   2.
      * `click` always renders arguments after `[OPTIONS]`, but you can put the options after the arg (before the next subcommand)
      * when using `click` arguments/options for a subcommand must be after the given group/subcommand, and before the next subcommand. `heron-explorer  --tracker-url http://127.0.0.1:8888 topologies local` won't work because _topologies_ is the subcommand with the `--tracker-url`, so you'd have to do `heron-explorer topologies local --tracker-url http://127.0.0.1:8888` or `heron-explorer topologies --tracker-url http://127.0.0.1:8888 local`.
   It felt a bit cleaner in the code to have `--tracker-url` and `--config-path` on the command, rather than the root level and having to pass them to the commands via the context dict
      * updated `--config-path` and `--tracker-url` to show the default, e.g.
           ```
           Usage: heron-explorer logical-plan [OPTIONS] CLUSTER[/ROLE[/ENV]] TOPOLOGY
           
             Show logical plan information for the given topology.
       
           Options:
             --config-path TEXT              Path to heron's config clusters config directory  [default: /home/vagrant/.heron/conf]
             --tracker-url TEXT              URL to a heron-tracker instance  [default: http://127.0.0.1:8888]
             --component-type [all|spouts|bolts]
                                             [default: all]
             --help                          Show this message and exit.
           ```
   3. Fixed `--component-type` filter not being applied
   
   ----
   re. the question, I was thinking of having both the CLI and web UI in the same binary, but I didn't think much about the static files - I think I just wanted to get rid of tornado in `heron.tools.common.src.python.access`. I'm not sure merging them would be a good idea now, although having all CLI functions in one binary (i.e. `heron`) sounds tempting to me


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org