Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

simplify remote execution #575

Merged
merged 5 commits into from
Apr 20, 2015

Conversation

rade
Copy link
Member

@rade rade commented Apr 19, 2015

Do as little as possible (and more obviously so) locally.

  • don't set vars we don't need
  • don't do any arg checking - leave that to the remote - except for run
  • disentangle handling of run
  • simplify dns_args

Closes #574.

rade added 3 commits April 19, 2015 13:19
This is desirable since we want to do as little as possible locally,
and not duplicate any checks.
for more logical grouping
@rade
Copy link
Member Author

rade commented Apr 19, 2015

There is less going on here than the overall diff indicates. It might make sense to review the individual commits.

The overall diff is larger than one might hope due to a) the need to move up the cidr validation functions, b) the move of the docker version validation code, and c) the removal of the condition around all the prerequisite checks, which really is just the removal of two lines and some leading whitespace but shows up as something far more complicated.

@rade
Copy link
Member Author

rade commented Apr 19, 2015

@squaremo this entire PR is mostly about changing bits introduced in your #388, so I'd appreciate you taking a look.

@rade rade added this to the 0.10.0 milestone Apr 19, 2015
@rade
Copy link
Member Author

rade commented Apr 19, 2015

before (i.e. on master)

$ sh -x weave launch |& wc -l
21
$ sh -x weave run 10.2.0.1/24 -ti ubuntu |& wc -l
69

after (i.e. on this branch)

$ sh -x weave launch |& wc -l
15
$ sh -x weave run 10.2.0.1/24 -ti ubuntu |& wc -l
48

@rade rade force-pushed the 574_simplify_remote_execution branch from 8121358 to a003d47 Compare April 19, 2015 13:08
rade added 2 commits April 20, 2015 11:30
pull all the local `weave run` execution into one place, thus
dispensing with the need to have lots of tests for whether we are
executing in the context of "--local".
Also, we only remote exec dns-args if "--with-dns" is supplied, which
is more efficient in the common case (and also more obvious).

NB: we need to retain the docker version check for `run` since the
specific breakage it guards against does affect the code we execute
locally for `run`.
it makes more sense to strip off the `--with-dns` at the call site.
`weave dns-args` now does something useful, instead of having to be
invoked as `weave dns-args --with-dns`.
@rade rade force-pushed the 574_simplify_remote_execution branch from a003d47 to b4c0005 Compare April 20, 2015 10:31
@squaremo squaremo self-assigned this Apr 20, 2015
@squaremo squaremo merged commit b4c0005 into weaveworks:master Apr 20, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

simplify remote execution
2 participants