interactive shell built on prompt_toolkit and tmuxp
Details
Details
- Reviewers
thcipriani dduvall demon - Group Reviewers
Release-Engineering-Team - Patch without arc
- git checkout -b D6 && curl -L https://phabricator.wikimedia.org/D6?download=true | git apply
run bin/iscap
Diff Detail
Diff Detail
- Repository
- rMSCA Scap
- Branch
- iscap-rebase
- Lint
Lint Errors Excuse: mozprocess: not my chair, not my problem - Unit
No Unit Test Coverage - Build Status
Buildable 128 Build 129: test harbormaster with jenkins Build 128: arc lint + arc unit
Event Timeline
| scap/prompt/context.py | ||
|---|---|---|
| 2 | This entire file is pretty convoluted and I want to clean it up substantially, however, it works... | |
Comment Actions
Gave some basic review here, started to go more in depth, but ran out of day. One weird thing that may be a prompt-tool kit bug: options only autocompleted for me on the first run so: typing Deploy shows completions for -s or -l, etc. then I ran: Deploy -l scap*01, then typing: Deploy again doesn't show any of the completions.
| scap/main.py | ||
|---|---|---|
| 916 ↗ | (On Diff #32) | keep. config_deploy stuff. |
| 932 ↗ | (On Diff #32) | keep. config deploy stuff. |
| 947 ↗ | (On Diff #32) | part of config_deploy patch too. If the config option is False, we don't want to pass it since python 'cause "truthy" |
| scap/monitor.py | ||
| 1 ↗ | (On Diff #32) | Maybe break this file into a separate diff. |
| 38 ↗ | (On Diff #32) | FWIW, you could use the requests library here. |
| 48 ↗ | (On Diff #32) | probably ought to use some kind of logging |
| scap/prompt/context.py | ||
| 4 | imported but unused | |
| 10 | Imported but unused | |
| 114 | unintuitive method name, might be a good use of an @property. Interface would then be: context.cmd = 'cd /home/thcipriani'; context.cmd.run() | |
| 117 | doesn't provide any shell expansions. cd ~ or cd dep* etc. | |
| 137 | probably should use a more explicit exception | |
| 180 | no need for an @property and @cwd.setter if you're just setting self._cwd, just change the variable name to self.cwd and you should be able to get it and set it. | |
| 282 | probably should use a more explicit exception | |
| 320 | probably should use a more explicit exception | |
| scap/ssh.py | ||
| 161 | currently self._run_with_reporter uses cluster_ssh and if no reporter is attached then cluster_ssh_threaded is run. This difference in behavior is non-obvious based on the fact that the decision which of the cluster methods to run is based on calling .progress() on an and instance of the Job class. | |
| 278 | This should utilize a logger, particularly since deploy-local now returns json. | |
| scap/tasks.py | ||
| 98 | log_context decorator requires logger argument | |
| 175–176 | log_context decorator requires logger argument | |
| 251 | log_context decorator requires logger argument | |
| 329 | Needs to be removed if you're removing the logger argument, log_context decorator requires logger argument | |
| 406 | requires logger keyword argument. Also, log_context is removed elsewhere. | |
| 450 | log_context decorator requires logger argument | |
| 613 | this code seems to be in a comment. | |
| setup.py | ||
| 26 | also needed:
| |
| scap/tasks.py | ||
|---|---|---|
| 98 | I think we should just get rid of the logger kwarg in log_context, I'll fix that in an updated diff coming soon... | |
Comment Actions
- iscap gains ability to run scap cli.Applications
- more logging cleanup
- fix rebase/merge errors and lint errors.
- testing jenkins + harbormaster