Remove --python3 option #5100

amanda11 · 2020-12-03T15:28:20Z

Remove the --python3 option and associated python3_... config items.

amanda11 · 2020-12-03T15:29:36Z

Waiting for U16 to move to python3, and is also dependant on fixes in progress for cryptography and using python3 on ST tests.

blag

Keep backwards compatibility for the CLI, since we haven't been emitting deprecation warnings for that. We can emit a warning when it is passed in, but we don't need to pass it to the ST2 API.

st2client/st2client/commands/pack.py

st2client/st2client/models/core.py

arm4b · 2020-12-15T13:21:16Z

@blag We announced that we're removing python2 completely and users had that notice and time to migrate. This means we're removing all the python2-related functionality.

I don't think keeping that old flag would help anymore, considering this behavior doesn't work and would just provide erroneous results. I think that'll bring confusion and false sense of a working feature.

We had enough deprecation Warnings about full Python2 removal via:

st2 startup logs
via st2ctl cmd
on pack install

This includes entire platform python2 feature set.

amanda11 · 2020-12-15T14:18:48Z

The cli did have a warning. St2 pack install had a warning in 3.3 if you installed a python 2 pack.

blag · 2020-12-15T20:12:12Z

If my understanding is correct, then there's two warnings that should have been generated:

Warning about installing Python 2-only pack
Warning about --python3 flag being deprecated and removed, since ST2 is going Python 3 only (unless you use the --force flag)

We definitely warned about Python 2 packs, but did we also warn about the removal of the --python3 flag? I don't see it, but if I missed it, please let me know.

Consider what people would have done in the past to ensure that their packs were always Python 3:

Their packs were Python 2 --> not good.
Update their packs to support Python 2 and Python 3 --> better.
Install their packs with --python3 in their deployment scripts or whatever --> awesome.

So now their pack still has to be installed with the --python3 flag, otherwise it gets installed with Python 2.

Since we defaulted to installing packs as Python 2 for so long, people who are trying to do things right have been forced into using the --python3 flag to ensure that their packs use Python 3.

If we up and remove this flag, the CLI will no longer recognize the flag and throw an error. That will cause those people's deployment scripts or whatever to immediately break. That's pretty rude of us - we are effectively penalizing people who have been doing things "right" and trying to switch to Python 3 for awhile.

Instead, we should at least allow a grace period where we still accept that (now superfluous) --python3 flag, but we emit a warning about it. That will give our users, who have been doing things the Right Way (tm) so far, time to remove the use of that flag from their deployment scripts.

If I have been operating under a misconception please correct me, but as I understand it, we should not break things for our best users.

amanda11 · 2020-12-15T20:18:36Z

I think your understanding is correct, and I get your point as to why we should allow them to still use --python3 option with a warning.
I am happy to rework as per your suggestion, if @armab also happy.

arm4b · 2020-12-25T13:15:06Z

@amanda11 Per https://stackstorm-community.slack.com/archives/CSBELJ78A/p1608157605269500 discussion, I'm 👍 fine with @blag plan to keep the flag for one more release if there are concerns that python 2 removal/deprecation communication wasn't enough for this change.

Conflicts: CHANGELOG.rst

amanda11 · 2020-12-28T15:01:53Z

@blag @armab I've kept in the --python3 option in the CLI, but I only kept it in the commands/pack.py. I didn't see any other print etc being done in the manager files, so I wasn't sure why we wanted to pass the argument through to the manager and do the prints in there, as all the other printing seems to be done in the commands/*.py files.
If it's needed I will move it to the manager file, but I wasn't sure on that suggestion.

Feedback appreciated on the code and warning text.

Example output:

$ st2 pack install aws_boto3 --python3

For the "aws_boto3" pack, the following content will be registered:

actions   |  6
rules     |  0
sensors   |  0
aliases   |  0
triggers  |  0

Installation may take a while for packs with many items.

DEPRECATION WARNING: --python3 flag will be ignored, as ST2 now runs with python3 as the default on all OS


	[ succeeded ] init_task
	[ succeeded ] download_pack
	[ succeeded ] make_a_prerun
	[ succeeded ] get_pack_dependencies
	[ succeeded ] check_dependency_and_conflict_list
	[ succeeded ] install_pack_requirements
	[ succeeded ] get_pack_warnings
	[ succeeded ] register_pack

+-------------+-------------------------+
| Property    | Value                   |
+-------------+-------------------------+
| ref         | aws_boto3               |
| name        | aws_boto3               |
| description | AWS actions using boto3 |
| version     | 0.8.0                   |
| author      | StackStorm, Inc.        |
+-------------+-------------------------+

arm4b · 2020-12-28T15:11:39Z

st2client/st2client/commands/pack.py

@@ -210,7 +210,10 @@ def run(self, args, **kwargs):
        if not is_structured_output:
            self._get_content_counts_for_pack(args, **kwargs)

-        return self.manager.install(args.packs, python3=args.python3, force=args.force,
+        if args.python3:
+            print('\nDEPRECATION WARNING: --python3 flag will be ignored, as ST2 now runs with python3 as the default on all OS\n')


Maybe something along the lines would be more precise?

DEPRECATION WARNING: --python3 flag is ignored and will be removed in v3.5.0 as StackStorm now runs with python3 only

blag · 2020-12-28T21:35:49Z

st2client/st2client/commands/pack.py

@@ -210,7 +210,10 @@ def run(self, args, **kwargs):
        if not is_structured_output:
            self._get_content_counts_for_pack(args, **kwargs)

-        return self.manager.install(args.packs, python3=args.python3, force=args.force,
+        if args.python3:
+            print('\nDEPRECATION WARNING: --python3 flag is ignored and will be removed in v3.5.0 as StackStorm now runs with python3 only\n')


Any reason to not use warnings.warn here?

blag · 2020-12-28T21:39:21Z

Rerunning end-to-end tests with this to ensure that StackStorm/st2tests#196 didn't break anything.

amanda11 · 2020-12-28T23:36:47Z

@blag I fixed the lint problem as it also needed an import of warnings. Output using warnings is as follows:

IInstallation may take a while for packs with many items.
/home/centos/st2/st2client/st2client/commands/pack.py:215: UserWarning: 
DEPRECATION WARNING: --python3 flag is ignored and will be removed in v3.5.0 as StackStorm now runs with python3 only

  warnings.warn('\nDEPRECATION WARNING: --python3 flag is ignored and will be removed '

	[ succeeded ] init_task

Initial changes

087b067

pull-request-size bot added the size/XL PR that changes 500-999 lines. Consider splitting work into several ones that easier to review. label Dec 3, 2020

amanda11 marked this pull request as draft December 3, 2020 15:28

amanda11 changed the title ~~Initial changes~~ [WIP] Remove --python3 option Dec 3, 2020

amanda11 added this to the 3.4.0 milestone Dec 3, 2020

Fix errors in UT amendments

cb73f03

pull-request-size bot added size/L PR that changes 100-499 lines. Requires some effort to review. and removed size/XL PR that changes 500-999 lines. Consider splitting work into several ones that easier to review. labels Dec 4, 2020

amanda11 added 2 commits December 4, 2020 15:02

Minor comment updates

4e8c626

Merge branch 'master' into remove_python3_option

f7fecd1

amanda11 changed the title ~~[WIP] Remove --python3 option~~ Remove --python3 option Dec 14, 2020

amanda11 marked this pull request as ready for review December 14, 2020 14:09

amanda11 added 2 commits December 14, 2020 14:09

Merge branch 'master' into remove_python3_option

85f6f32

Update changelog

de323e3

blag suggested changes Dec 14, 2020

View reviewed changes

st2client/st2client/commands/pack.py Outdated Show resolved Hide resolved

st2client/st2client/commands/pack.py Show resolved Hide resolved

st2client/st2client/models/core.py Show resolved Hide resolved

Merge branch 'master' into remove_python3_option

129ff13

amanda11 added 2 commits December 28, 2020 14:53

Keep --python3 but omit warning

70cca57

Merge branch 'master' into remove_python3_option

4e40b6c

Conflicts: CHANGELOG.rst

arm4b reviewed Dec 28, 2020

View reviewed changes

Update deprecation warning message

bc7cb40

blag suggested changes Dec 28, 2020

View reviewed changes

blag and others added 3 commits December 28, 2020 15:18

Address linting issues

66c6e5d

Use warnings.warn

96b29ea

Add missing import

579fad6

blag mentioned this pull request Dec 28, 2020

TODO: Remove the --python3 flag and warning in packs.install #5110

Closed

Newlines not needed with warnings.warn

48c4150

blag approved these changes Jan 6, 2021

View reviewed changes

amanda11 merged commit 71e0cdf into master Jan 6, 2021

amanda11 deleted the remove_python3_option branch January 6, 2021 11:29

blag mentioned this pull request Jan 26, 2021

Packs that have actions in sub-directories importing modules in same sub-directory fail #5126

Closed

blag mentioned this pull request Feb 8, 2021

Rescue Python path code #5142

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove --python3 option #5100

Remove --python3 option #5100

amanda11 commented Dec 3, 2020

amanda11 commented Dec 3, 2020

blag left a comment

arm4b commented Dec 15, 2020

amanda11 commented Dec 15, 2020

blag commented Dec 15, 2020

amanda11 commented Dec 15, 2020

arm4b commented Dec 25, 2020 •

edited

Loading

amanda11 commented Dec 28, 2020 •

edited

Loading

arm4b Dec 28, 2020

blag Dec 28, 2020

blag commented Dec 28, 2020

amanda11 commented Dec 28, 2020 •

edited

Loading

Remove --python3 option #5100

Remove --python3 option #5100

Conversation

amanda11 commented Dec 3, 2020

amanda11 commented Dec 3, 2020

blag left a comment

Choose a reason for hiding this comment

arm4b commented Dec 15, 2020

amanda11 commented Dec 15, 2020

blag commented Dec 15, 2020

amanda11 commented Dec 15, 2020

arm4b commented Dec 25, 2020 • edited Loading

amanda11 commented Dec 28, 2020 • edited Loading

arm4b Dec 28, 2020

Choose a reason for hiding this comment

blag Dec 28, 2020

Choose a reason for hiding this comment

blag commented Dec 28, 2020

amanda11 commented Dec 28, 2020 • edited Loading

arm4b commented Dec 25, 2020 •

edited

Loading

amanda11 commented Dec 28, 2020 •

edited

Loading

amanda11 commented Dec 28, 2020 •

edited

Loading