Tidy e2e test Python environment

[antonymilne]

There's various things that are pip-installed in environment.py. Ideally we shouldn't be putting custom requirements in here. I tried removing them in #804 but it started failing some tests - looks like we need the click specification for testing on older version of kedro 0.16.

Given that the kedro requirements are going to continually keep bumping the version of click we should work out some better way of installing requirements here so that the testing environment is a clean as possible. Or maybe we should just forget about tests for 0.16 altogether?

There's probably some other bits of tidying that would be useful here, e.g. setting PIP_DISABLE_PIP_VERSION_CHECK would be much nicer if done in the CI config, as we do on kedro.

[tynandebold]

We can now let testing of version 16 fall away and only test back one minor version, so 0.17.

[antonymilne]

Even better, after doing #812 we should have e2e and unit tests split into separate jobs. Then it might be possible to completely remove https://github.com/kedro-org/kedro-viz/blob/main/package/features/environment.py and not use any venv at all.

[studioswong]

@AntonyMilneQB would it make a difference if we are supporting >= 0.16.6 instead of >=0.16.1 ( as I saw from the tests in viz.feature?) The main reason being that we have still been supporting >=0.16.6 since 3.8.0 and was thinking it would be good to at last support the slightly older versions of Kedro so that they could still get any bug fixes we put for the flowchart features. ( there might still be a big group of users on older CSTs that might not have made the move to upgrade to 0.17 yet)

If we are dropping support for 0.16 altogether, this would be worth a discussion with the wider team as that would trigger some other actions, such as notifying the user groups of the drop our support for 0.16 altogether, etc.

[antonymilne]

@studioswong Very fair point. As far as I can tell...

• kedro-viz has specified kedro>=0.16.0 since kedro-viz 3.8.0 (here's the commit: https://github.com/kedro-org/kedro-viz/commit/add5c717deed6a1d04abff5aad12c0cffe5067a4)
• we have special logic to handle kedro < 0.17.0 but it's the same for all 0.16.x releases, so nothing special about kedro 0.16.6 compared to 0.16.0 or any other 0.16.x release
• not sure why the e2e test in viz.feature uses kedro 0.16.1 but I don't think it would make any difference if this was any other release of 0.16.x
• there's nothing in kedro 0.18 which means we need to drop support for kedro 0.16 in kedro-viz
• when @rashidakanchwala was working on support for Python 3.9 and 3.10 she added back CI tests for Python 3.7, and that gave some problem for the kedro 0.16 e2e test...
• ... BUT, although I didn't look into this in detail, at a glance it should be fairly easy to fix I think

So actually I don't think it would be difficult to continue to support kedro 0.16, assuming the e2e test is easy to fix. At some point in the future it will probably become a burden to maintain compatibility with some older versions of kedro, but at the moment I think it's ok actually.

I guess there's two questions here actually:

1. should the "officially supported kedro versions" be the same as "kedro versions we test for"? I think probably yes, it makes sense to keep these in sync. So if we do drop tests for 0.16 then we should bump the kedro requirement in requirements.txt, even if the latest kedro-viz does still work for kedro 0.16.x. Alternatively we can just drop the tests and not bump the requirements, i.e. we don't care if kedro-viz still works for kedro 0.16.x; currently it does and you can use it if you like, but there's no guarantee it will continue working
2. what should the above versions of kedro be? Personally I don't have strong feelings here. I don't think it would be a big effort to continue to support 0.16.x, but at some point it will make sense to drop it I expect

Edit: I see that @studioswong point about 0.16.6 comes from this comment in our contributors guide. Not disputing the correctness of that statement at all, but I don't know why it would be true - would need to look into it further. Assuming we are actually only compatible with kedro >= 0.16.6 and not >=0.16.0 then dropping support for 0.16.x would be a smaller change if we don't support <0.16.6 already.

[antonymilne]

Side note: whatever we do with 0.16.x support, we should probably make a new e2e test for 0.17, since the current e2e test will just test latest kedro 0.18.

[studioswong]

@AntonyMilneQB thanks for the detailed outline of this situation! I 100% agree on the point that the "officially supported kedro versions" be the same as "kedro versions we test for".

That said, @idanov made a fair point in standup yesterday that we are supporting 2 versions of Kedro, which, given the upgrade to 0.18, it does make sense to drop support for 0.16. Also noted on the fact that Kedro 0.16 causes tests to fail on Python 3.7 and it makes sense to drop the support altogether.

Great that you brought up about this comment on the contributing guide - it's been a while since this issue was discussed ( 14 months ago - time flies!), the main context behind this lies from a user report that mentioned Kedro-Viz not working well with older versions of Kedro ( i.e Kedro <=0.16.6), where back then we did a bunch of tests within the team ourselves to test with different Kedro 0.16 versions and concluded that Kedro-Viz will work with 0.16.6 onwards. ( you can refer to this PR here for some context, while a lot of the discussion also happened on slack).

You're right in that it's not a set in stone statement, it was just a result from testing where we can safely advise users that Kedro-Viz >=3.8 will work with Kedro>=0.16.6 at that time and we made the releavant announcement about this on the kedro-user channel. If we are dropping support for Kedro 0.16 altoegther, it would be good to remind our users that as well so we can prompt them to upgrade to get the latest Viz goodies ( we should definitely keep in mind to include the statement that we are dropping support for Kedro <0.17.0 in our next release announcement)

[tynandebold]

Closing in favor of #967, which will solve this.

[antonymilne]

@tynandebold I've re-opened this because I think the Python e2e tests still need work, even if we drop support for 0.16 - hope that's ok.

• we should make a new e2e test for 0.17, since the current e2e test will just test latest kedro 0.18.
• now that we've split up the unit and e2e tests on CI it might be possible to completely remove https://github.com/kedro-org/kedro-viz/blob/main/package/features/environment.py and not use any venv at all for e2e tests
• if we can't completely remove it, we should tidy it up to e.g. remove the custom pip installed requirements as much as possible, move PIP_DISABLE_PIP_VERSION_CHECK to CI config, etc.