Skip to content

cluster management#298

Merged
kwmsmith merged 19 commits into
enthought:masterfrom
cowlicks:cli
Apr 15, 2014
Merged

cluster management#298
kwmsmith merged 19 commits into
enthought:masterfrom
cowlicks:cli

Conversation

@cowlicks
Copy link
Copy Markdown
Contributor

@cowlicks cowlicks commented Apr 9, 2014

This moves the entry point of cluster management from the Makefile to dacluster. When distarray is installed it should be available. Working commands are start, stop, clear and purge.

For example you can now run:

dacluster clear

from anywhere, not just in the Makefile directory.

@bgrant bgrant added this to the 0.3 milestone Apr 10, 2014
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Encoding statement, copyright...

@bgrant
Copy link
Copy Markdown
Contributor

bgrant commented Apr 10, 2014

Pretty cool. It would be nice if we could pass in the number of engines to start and restart, and if the subcommands had help. I see I can run subcommand help , it just doesn't have much in it yet.

$ dacluster start --help
usage: dacluster start [-h]

optional arguments:
  -h, --help  show this help message and exit

I get a little bit of weird output with dacluster --help---a spurious "subparsers".

$ dacluster --help
usage: dacluster [-h] {start,stop,restart,clear,purge} ...

Start, stop and manage a IPython.parallel cluster. `dacluster` can take all
the commands IPython's `ipcluster` can, and a few extras that are distarray
specific.

positional arguments:
  {start,stop,restart,clear,purge}
                        subparsers

optional arguments:
  -h, --help            show this help message and exit

@bgrant
Copy link
Copy Markdown
Contributor

bgrant commented Apr 10, 2014

Any reason not to add dump as well?

This is probably more flexible than the Makefile already though (except for dump), so most of the above is gravy.

@cowlicks
Copy link
Copy Markdown
Contributor Author

@bgrant okay I think I got all that. There are no unittests for this so you might want to fetch it and try it out again.

@cowlicks
Copy link
Copy Markdown
Contributor Author

Oops, I unkowingly included a Py3 feature. The future is now!

Comment thread distarray/apps/dacluster.py Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"This has a few leaks" -- what do you mean?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It leaks keys, I'll make it more specific.

@kwmsmith
Copy link
Copy Markdown
Contributor

For simplicity, let's merge the ipcluster_tools.py and purge_cluster.py into dacluster.py -- I see no need to keep things separate since dacluster.py is the main entry point. That way we can remove duplicated logic for commandline parsing.

Comment thread distarray/apps/dacluster.py Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. It would be nice if it said something like, "for details on a subcommand, try dacluster <subcommand> --help.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@bgrant
Copy link
Copy Markdown
Contributor

bgrant commented Apr 14, 2014

Another "would be nice". It would be nice if it printed out a nicer error for bad subcommands.

$ dacluster spam
Traceback (most recent call last):
  File "/Users/robertgrant/development/venvs/py27/bin/dacluster", line 9, in <module>
    load_entry_point('distarray==0.2.0', 'console_scripts', 'dacluster')()
  File "/Users/robertgrant/development/cowlicks-distarray/distarray/apps/dacluster.py", line 99, in main
    args = parser.parse_args()
  File "/Users/robertgrant/development/venvs/py27/lib/python2.7/argparse.py", line 1688, in parse_args
    args, argv = self.parse_known_args(args, namespace)
  File "/Users/robertgrant/development/venvs/py27/lib/python2.7/argparse.py", line 1727, in parse_known_args
    self.error(str(err))
  File "/Users/robertgrant/development/cowlicks-distarray/distarray/apps/dacluster.py", line 24, in error
    ipcluster_tools.run_ipcluster(sys.argv[1:])
  File "/Users/robertgrant/development/cowlicks-distarray/distarray/apps/ipcluster_tools.py", line 32, in run_ipcluster
    Popen(command, stdout=PIPE, stderr=PIPE)
  File "/Users/robertgrant/development/venvs/py27/lib/python2.7/subprocess.py", line 711, in __init__
    errread, errwrite)
  File "/Users/robertgrant/development/venvs/py27/lib/python2.7/subprocess.py", line 1184, in _execute_child
    args = list(args)
TypeError: 'NoneType' object is not iterable

@cowlicks
Copy link
Copy Markdown
Contributor Author

@kwmsmith I merged purge_cluster.py into ipcluster_tools.py. While the functions in ipcluster_tools.py are not the most generic, I think it might be worthwhile to keep them separate from dacluster.py in case we want to use them in another script.

@bgrant that feature is now working.

@kwmsmith
Copy link
Copy Markdown
Contributor

@cowlicks For now, I think encapsulating all of this in one module makes sense. Everything is related to managing the clusters and engine namespaces, so they're related. We have no usecase for separating out the ipcluster_tools functions currently -- let's wait for one to come along to drive splitting that out.

@cowlicks
Copy link
Copy Markdown
Contributor Author

@kwmsmith done.

@kwmsmith
Copy link
Copy Markdown
Contributor

@cowlicks this is nice -- the only downside is that I don't have tab completion with the makefile anymore, though, but I think I can live with that...

kwmsmith added a commit that referenced this pull request Apr 15, 2014
@kwmsmith kwmsmith merged commit ac98c46 into enthought:master Apr 15, 2014
@cowlicks cowlicks deleted the cli branch April 15, 2014 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants