Skip to content

ignore py2 errors - #381

Merged
ansible-zuul[bot] merged 1 commit into
ansible-collections:mainfrom
goneri:ignore-py2-errors_22571
Jun 2, 2021
Merged

ignore py2 errors#381
ansible-zuul[bot] merged 1 commit into
ansible-collections:mainfrom
goneri:ignore-py2-errors_22571

Conversation

@goneri

@goneri goneri commented Jun 2, 2021 •
edited
Loading

Copy link
Copy Markdown
Member
  • ignore all the errors with pre-3.8 Python
  • add a script to refresh the ignores files easily

@ansibullbot

ansibullbot commented Jun 2, 2021

Copy link
Copy Markdown

@goneri: Greetings! Thanks for taking the time to open this pullrequest. In order for the community to handle your pullrequest effectively, we need a bit more information.

Here are the items we could not find in your description:

  • issue type

Please set the description of this pullrequest with this template:
https://raw.githubusercontent.com/ansible/ansible/devel/.github/PULL_REQUEST_TEMPLATE.md

click here for bot help

@ansibullbot ansibullbot added needs_info This issue requires further information. Please answer any outstanding questions needs_template This issue/PR has an incomplete description. Please fill in the proposed template correctly needs_triage tests tests labels Jun 2, 2021
@tremble

tremble commented Jun 2, 2021

Copy link
Copy Markdown
Contributor

Can we use the --skip-test option instead of mass-adding ignore entries?

@goneri
ignore py2 errors
39f483b 
- ignore all the errors with pre-3.8 Python
- add a script to refresh the ignores files easily
@goneri
goneri force-pushed the ignore-py2-errors_22571 branch from ce6d6f2 to 39f483b Compare June 2, 2021 14:38
@goneri

goneri commented Jun 2, 2021 •
edited
Loading

Copy link
Copy Markdown
Member Author

Can we use the --skip-test option instead of mass-adding ignore entries?

We cannot use --skip-test, the option does not accept things like compile-3.7!skip. It can just fully turn off the test. In addition, we don't control the ansible-test configuration of AutomationHub. So it will still test againt py27.

@goneri
goneri requested a review from jillr June 2, 2021 14:45
@ansibullbot ansibullbot added community_review and removed needs_info This issue requires further information. Please answer any outstanding questions needs_template This issue/PR has an incomplete description. Please fill in the proposed template correctly labels Jun 2, 2021
@goneri goneri mentioned this pull request Jun 2, 2021
Merged
@tremble

tremble commented Jun 2, 2021

Copy link
Copy Markdown
Contributor

@goneri Then FWIW this is probably the right change to make but we should talk to @mattclay about a better long term solution for disabling/reconfiguring ansible-test

@tremble

tremble commented Jun 2, 2021

Copy link
Copy Markdown
Contributor

https://github.com/openstack/ansible-collections-openstack/blob/master/tools/run-ansible-sanity.sh#L28

I don't quite get why we care about not totally turning off the Python 2.6/2.7/3.5 tests, but I'll not push here.

@jillr

jillr commented Jun 2, 2021

Copy link
Copy Markdown
Contributor

@tremble I would also prefer to avoid hundreds of ignores (community.aws is going to be "fun") but in the immediate term we can't keep blocking CI. :(

@goneri

goneri commented Jun 2, 2021

Copy link
Copy Markdown
Member Author

https://github.com/openstack/ansible-collections-openstack/blob/master/tools/run-ansible-sanity.sh#L28

I don't quite get why we care about not totally turning off the Python 2.6/2.7/3.5 tests, but I'll not push here.

They test with Python 3.6 only. This is another approach that we can take, but we will still need to have the ignore files at some point because of AutomationHub.

jillr
jillr approved these changes Jun 2, 2021
View reviewed changes

@jillr jillr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm when #370 passes (contains 3-only changes)

@goneri

goneri commented Jun 2, 2021

Copy link
Copy Markdown
Member Author

@tremble I've open this a while ago: ansible/ansible#74398. @mattclay has since pushed a new feature that give the ability to disable a Python version. But it won't be backported.

@goneri goneri mentioned this pull request Jun 2, 2021
Merged
goneri added a commit to goneri/community.aws that referenced this pull request Jun 2, 2021
@goneri
ignore failure with old Python
1f40cda 
We only support Python 3.8 now. This commit had:

- a script to refresh the ignore files:
  tests/sanity/refresh_ignore_files
- the ignore files.

See: ansible-collections/amazon.aws#381
@jillr

jillr commented Jun 2, 2021

Copy link
Copy Markdown
Contributor

370 sanity is passing now (the integration failures are unrelated). thanks @goneri!

@jillr jillr added the gate label Jun 2, 2021
@ansible-zuul
ansible-zuul Bot merged commit 5bc1292 into ansible-collections:main Jun 2, 2021
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
@goneri
ignore failure with old Python
551d147 
We only support Python 3.8 now. This commit had:

- a script to refresh the ignore files:
  tests/sanity/refresh_ignore_files
- the ignore files.

See: ansible-collections/amazon.aws#381
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
@goneri
ignore failure with old Python
61f991c 
We only support Python 3.8 now. This commit had:

- a script to refresh the ignore files:
  tests/sanity/refresh_ignore_files
- the ignore files.

See: ansible-collections/amazon.aws#381
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jillr jillr jillr approved these changes

Assignees

No one assigned

Labels

community_review needs_triage tests tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@goneri @ansibullbot @tremble @jillr