Easy creation of v2+ Connection objects from v1 'env' module state

[bitprophet]

This is another "make it easier to upgrade to v2 piecemeal" feature: with fabric and fabric2 both imported, create a fabric2.Connection from one's current fabric.state.env contents.

TODO

• Port over all low hanging fruit env vars so it mostly Just Works for the average user
• Take another whack at testing+implementing automatic 'from fabric.api import env', or roll it back in the docs if it's just not feasible to test.
• Try real quick to add a Travis-oriented task or script line, which installs v1 and v2 side by side to do an integration level test of this functionality (regardless of whether we implemented automatic importing; can just do it explicitly.)

Thoughts

Brainstorm re: appropriate API:

• Presumably wants to be a classmethod-constructor on Connection
  • .from_v1? bit on the nose/awkward but very clear/accurate
  • .from_env? easily mistaken for other more general senses of "env"...
  • .from_module? ditto, could mean a lot of things...
  • .from_dict? (actually more accurate than from_module as env is a dict subclass, just one that is a singleton _in_a module) - not amazing but not awful
  • .from_env_dict
  • .from_v1_env - nicely explicit? slightly more obvious than just .from_v1 too?
  • .import? since we're "importing" v1 state?
• Arguments?
  • None - implicitly does its own from fabric.api import env? Convenient but very fragile
  • Env obj - you do Connection.from_v1_env(env) and we don't care how you imported it? Less fragile, slightly more work depending on how often users are calling it.
  • Both? No real reason why not...having the ultra-convenient no-args version seems like a big win for removing friction.
  • Any behaviors we want toggle-able/overrideable at this level? Easy enough to add later...

Thoughts as I run down env:

• Going to focus on just basic Connection vars for right now, the rest can remain TODOs
• Most of those other settings probably want to end up in a Config; implies we may want a Config.from_v1 to accompany this?
  • You'd still potentially need to call both, though could probably state that Connection.from_v1 implicitly calls Config.from_v1 internally by default?
• Any defaults from v1 that aren't properly None or other "empty, not explicitly false" values will be problematic, e.g. use_ssh_config==False should not necessarily be interpreted as "user wants SSH config files to not be loaded".
  • Especially annoying for settings that v2 inverted the effective default of, like SSH conf loading
• Some not listed below are skipped because they represent 'missing' features from v2; once those features are ported/implemented, we could expand this.

env vars of interest:

• host_string, just host
• host_string, user too
• host_string, port too
• host_string, all 3
• always_use_pty, would set run.pty
• forward_agent can just map to the same exact kwarg
• gateway can just map to the same exact kwarg, it's a host-string
• key wants to become connect_kwargs.pkey - lol nope whoops v1 does extra work here we do not want to replicate
• key_filename -> connect_kwargs.key_filename
• keepalive probably also goes into connect_kwargs? I forget. - v2 doesn't support it yet
• hosts might want to be used by, say, a Group.from_v1? - that wants to wait for improved roles in v2 for sure
• no_agent -> not connect_kwargs.allow_agent
• password -> probably connect_kwargs.password only? since in Paramiko it's backwards compat too?
• passwords -> yank out the value mapping to current host_string? - wait for per-host config feature
• port -> straight to port kwarg
• ssh_config_path -> same name in v2, at Config level (is not a Cxn kwarg)
• sudo_password -> sudo.password
• sudo_passwords -> yank out the value mapping to current host_string? - wait for per-host config feature
• sudo_prompt -> sudo.prompt
• timeout -> timeouts.connection
• use_ssh_config -> config
• user -> user kwarg
• warn_only -> run.warn

[bitprophet]

Been using this for a while internally with just host-string/key-filename; time to polish it up and release it.

Want to get more of the checklist ticked off, though I am still going to punt on anything too complex. Intent is to have it Just Work for the average user while not spending forever in a rabbit hole.

[bitprophet]

Been using this for a while internally with just host-string/key-filename; time to polish it up and release it.

Want to get more of the checklist ticked off, though I am still going to punt on anything too complex. Intent is to have it Just Work for the average user while not spending forever in a rabbit hole.

[bitprophet]

Some boolean settings whose values flipped in v2, like always_use_pty and use_ssh_config, are problematic:

• Their defaults in v1 were True/False and not None, meaning we cannot tell if a user actually wanted them set or left them untouched. Thanks, past me.
• Because the new defaults in v2 are different, the one mutation we can detect - that a v1 user set, for example, always_use_pty = False - doesn't do anything useful because...that mutated value is the v2 default anyways!

My initial thought here was to just mark these as "not supported" and not port them, because I assumed the obvious desired behavior here is to move to the new, v2 default behaviors (e.g. pty off, ssh config on).

However I'm now wondering: maybe USING the v1 defaults makes more sense? If one is truly attempting to get a v2 Connection obj that "feels like" their v1 setup, they'd WANT whatever the v1 defaults were?

A counter-counterpoint might be that they'll be set up for a nasty surprise once they finally do away with their v1 code and switch from Connection.from_v1() to Connection() (or task contexts handed in by Executor, or etc) because at that point, their assumptions about pty or SSH config behavior will be violated.

I think given that I'm torn, it still makes more sense to skip those for the time being, and then if enough users say they felt that preserving that behavior was what they were expecting, we can "add" them in. It's harder to remove it in the opposite scenario.

[bitprophet]

Some boolean settings whose values flipped in v2, like always_use_pty and use_ssh_config, are problematic:

• Their defaults in v1 were True/False and not None, meaning we cannot tell if a user actually wanted them set or left them untouched. Thanks, past me.
• Because the new defaults in v2 are different, the one mutation we can detect - that a v1 user set, for example, always_use_pty = False - doesn't do anything useful because...that mutated value is the v2 default anyways!

My initial thought here was to just mark these as "not supported" and not port them, because I assumed the obvious desired behavior here is to move to the new, v2 default behaviors (e.g. pty off, ssh config on).

However I'm now wondering: maybe USING the v1 defaults makes more sense? If one is truly attempting to get a v2 Connection obj that "feels like" their v1 setup, they'd WANT whatever the v1 defaults were?

A counter-counterpoint might be that they'll be set up for a nasty surprise once they finally do away with their v1 code and switch from Connection.from_v1() to Connection() (or task contexts handed in by Executor, or etc) because at that point, their assumptions about pty or SSH config behavior will be violated.

I think given that I'm torn, it still makes more sense to skip those for the time being, and then if enough users say they felt that preserving that behavior was what they were expecting, we can "add" them in. It's harder to remove it in the opposite scenario.

[bitprophet]

I keep waffling though. Writing out the explanation for this in the docs just feels bad and overly complicated.

What are the use cases in my head for this? My default one was my own: writing new v2-using code inside a legacy codebase (to avoid making more work for a planned future migration of existing code to v2). In this scenario, I want things like pty/ssh_config to "feel like" v2, because it is new code I am writing from scratch.

But I bet at least as many, if not more, users would use this to port existing v1 using code to v2, for example turning this:

run("blah")
sudo("seriously, please blah")
put("file")

into this:

c = Connection.from_v1()
c.run("blah")
c.sudo("seriously, please blah")
c.put("file")

in which case, having c feel just like v1 might be initially preferable, because it's being applied to pre-existing code which is written against the v1 config - including ptys and (lack of) ssh configs.

So I'm gonna go with "rock the boat less" for now.

[bitprophet]

I keep waffling though. Writing out the explanation for this in the docs just feels bad and overly complicated.

What are the use cases in my head for this? My default one was my own: writing new v2-using code inside a legacy codebase (to avoid making more work for a planned future migration of existing code to v2). In this scenario, I want things like pty/ssh_config to "feel like" v2, because it is new code I am writing from scratch.

But I bet at least as many, if not more, users would use this to port existing v1 using code to v2, for example turning this:

run("blah")
sudo("seriously, please blah")
put("file")

into this:

c = Connection.from_v1()
c.run("blah")
c.sudo("seriously, please blah")
c.put("file")

in which case, having c feel just like v1 might be initially preferable, because it's being applied to pre-existing code which is written against the v1 config - including ptys and (lack of) ssh configs.

So I'm gonna go with "rock the boat less" for now.

[bitprophet]

Grumble, ran into a stumbling block: so far, I've been dealing with data that is pertinent to Connection itself, like user/host/port or connect_kwargs members.

But some things, like always_use_pty, sudo_password/prompt, etc only really impact either run() or the Config in play; Connection's instantiation has no handles for this type of setting.

Clearly, we cannot modify run this way, so we'd have to modify the Config. However, users may well want to pass in their own Config, at which point there's ambiguity about whether, say, an explicitly-configured ssh_config_path in a loaded Config should win over what's coming over from the v1 env. (Also whether to mutate such an env or make a copy.)

I seem to have missed writing it down, but I now recall earlier thinking we might need another analogue to Connection.from_v1 which is Config.from_v1. Then users who want to do things to a config on their own could call conf = Config.from_v1(), modify conf, then hand it to Connection.from_v1 -- and have Connection.from_v1 either call Config.from_v1 internally (if it was not given a config) or accept the given config without any modifications (if given one).

The only use case this doesn't seem to serve well is when users are trying to merge a config handed to them from elsewhere (eg a vanilla Invoke or host-less Fabric CLI+@task scenario) with their v1 environment. I think this will be sufficiently rare that it's not worth worrying about for now. Worst case we can grow something like config_instance.update_from_v1() that basically performs a mutation variant on the from_v1 classmethod.

[bitprophet]

Grumble, ran into a stumbling block: so far, I've been dealing with data that is pertinent to Connection itself, like user/host/port or connect_kwargs members.

But some things, like always_use_pty, sudo_password/prompt, etc only really impact either run() or the Config in play; Connection's instantiation has no handles for this type of setting.

Clearly, we cannot modify run this way, so we'd have to modify the Config. However, users may well want to pass in their own Config, at which point there's ambiguity about whether, say, an explicitly-configured ssh_config_path in a loaded Config should win over what's coming over from the v1 env. (Also whether to mutate such an env or make a copy.)

I seem to have missed writing it down, but I now recall earlier thinking we might need another analogue to Connection.from_v1 which is Config.from_v1. Then users who want to do things to a config on their own could call conf = Config.from_v1(), modify conf, then hand it to Connection.from_v1 -- and have Connection.from_v1 either call Config.from_v1 internally (if it was not given a config) or accept the given config without any modifications (if given one).

The only use case this doesn't seem to serve well is when users are trying to merge a config handed to them from elsewhere (eg a vanilla Invoke or host-less Fabric CLI+@task scenario) with their v1 environment. I think this will be sufficiently rare that it's not worth worrying about for now. Worst case we can grow something like config_instance.update_from_v1() that basically performs a mutation variant on the from_v1 classmethod.

[bitprophet]

A few days removed, I find that I implemented from_v1 such that data from the v1 env overwrites any kwargs given, intentionally (with a docs warning and test). However I don't entirely recall now why this was preferable over the obvious (kwargs win as they are closer to runtime/invocation).

Suspect it's to do with my internal usage needs or something, but regardless, it just seems so unintuitive & so hard to work around (how is a user supposed to slowly start migrating away from that v1 env for any settings that conflict?) I'm gonna try reversing it to see what, if anything, breaks on my end.

At the least, if that happens & I remember what made it seem sensible, I can record it in the warning block and here...

[bitprophet]

A few days removed, I find that I implemented from_v1 such that data from the v1 env overwrites any kwargs given, intentionally (with a docs warning and test). However I don't entirely recall now why this was preferable over the obvious (kwargs win as they are closer to runtime/invocation).

Suspect it's to do with my internal usage needs or something, but regardless, it just seems so unintuitive & so hard to work around (how is a user supposed to slowly start migrating away from that v1 env for any settings that conflict?) I'm gonna try reversing it to see what, if anything, breaks on my end.

At the least, if that happens & I remember what made it seem sensible, I can record it in the warning block and here...

[bitprophet]

OK, basically everything obvious has been set up to be pulled into Config.from_v1 and Connection.from_v1 (as expected it mostly ended up the former, not the latter. hooray?)

Going to just drop the automagic "does the import for you" for now, if people complain about needing one extra (or modified) import line and one explicit value reference, I feel like maybe they'd have trouble with almost any API?

[bitprophet]

OK, basically everything obvious has been set up to be pulled into Config.from_v1 and Connection.from_v1 (as expected it mostly ended up the former, not the latter. hooray?)

Going to just drop the automagic "does the import for you" for now, if people complain about needing one extra (or modified) import line and one explicit value reference, I feel like maybe they'd have trouble with almost any API?

[bitprophet]

After spending waaaay too much time wrestling with Travis, I got a v1-and-v2-at-same-time sanity check working there too. (You could say the sanity check cost my my sanity…)

This is ✨ DONE ✨

[bitprophet]

After spending waaaay too much time wrestling with Travis, I got a v1-and-v2-at-same-time sanity check working there too. (You could say the sanity check cost my my sanity…)

This is ✨ DONE ✨