Remove extra_env and corresponding test

[kevin-bates]

As mentioned here, the use of extra_env is essentially rogue code that came into existence as part of future work that later was removed from master (or something like that). These changes remove its use since its existence interferes with the use of the long-deprecated kernel_cmd trait.

Fixes #580

[willingc]

This looks good to me @kevin-bates. Will tag @MSeal for review too since he has more historical context.

[MSeal]

I'm good with removing this. I believe this is the pattern (

jupyter_client/jupyter_client/tests/test_kernelapp.py

Line 9 in 58e1971

def _launch(extra_env):

) used when env manipulation is done in other applications that I know of which need to do this pattern. We should remember to do a minor version bump next release since it is removing a capability.

[kevin-bates]

@willingc and @MSeal - thank you for the review and merge.

Matt, regarding...

We should remember to do a minor version bump next release since it is removing a capability.

I'm a little confused about what capability is being removed since extra_env was essentially invalid since its introduction into the master branch relative to the use of kernel_cmd. (In the kernel_discovery branch, from whence this came, extra_env is definitely a "capability".) I suppose some application could be accessing the kernel manager directly (as the removed test code had done) and explicitly setting both kernel_cmd and extra_env (which would be the only way use of kernel_cmd could be made to work), but I view this PR more as a bug fix than removal of functionality. Clearly, no one had ever tried using kernel_cmd post-6.x until now. Or is your comment more about the eventual removal of the 7-year deprecated kernel_cmd - which probably warrants further discussion?

I don't mind incrementing the minor version on the next release for this particular change but just wanted to understand what capability was being removed. Thanks.

[MSeal]

That's a decent argument for it being classified as a bug fix. I have found that if it exists in any capacity, someone somewhere has extended and repurposed it, so I tend to assume if I am deleting something that could be used it's a breaking change. Hence the minor version suggestion. Nothing beyond that on my thinking and it's not a strong opinion therein for this change.

[kevin-bates]

Ok - thank you for the explanation.

I have found that if it exists in any capacity, someone somewhere has extended and repurposed it

I agree totally agree and I'm guilty of that myself. 😄

Ok - let's move to 6.2 at next build just to be safe.