Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/stop checking unused boto versions #58715

Closed

Conversation

major0
Copy link
Collaborator

@major0 major0 commented Oct 12, 2020

What does this PR do?

Modifies the default behavior of salt.utils.check_boto_reqs() such that it stops defaulting the default assumption that all code needs boto and boto3.

The AWS boto2 and boto3 libraries are only compatible in regards to "some" method names (though not in their arguments to those methods), they do not use the same config patterns, data structures, or even the same exceptions.

The AWS boto3 library went stable 5 years ago, and the old boto2 (boto v2) library was deprecated shortly after. The last bugfix release for boto was 2 years ago and it is all but dead now. I have been working on a number of other PR's to try to remove the need for boto from SaltStack in part to even get SaltStack to fully work with the AWS configs and new features (E.g. ECS has moved to extended ARNs that are unavailable in boto2).

Previous Behavior

Previous version of salt.utils.check_boto_reqs() assumed that all code needed both boto_ver=2.0.0 and boto3_ver=1.2.6 as minimum versions, even if the code only ever used one or the other. Most boto modules include tests for both boto and boto3 as a result of this pattern even if all remaining code is strictly boto2 or boto3.

Of all the code in SaltStack I have only found 3 modules that make attempts to utilize boto2 and boto3 at the same time, and they contain comments similar to:

        # Using conn.get_response is a bit of a hack, but it avoids having to
        # rewrite this whole module based on boto3

New Behavior

  • Changes the default behavior of salt.utils.check_boto_reqs() to stop assuming boto modules need both boto2 and boto3.
  • Deprecate check_boto and check_boto3 parameters. The performance of any test should rely on the specification of a boto/boto3 version.
  • Modifies all boto/boto3 code to clearly identify which version of boto/boto3 it requires to run.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes

@major0 major0 requested a review from a team as a code owner October 12, 2020 15:50
@major0 major0 requested review from waynew and removed request for a team October 12, 2020 15:50
@ghost ghost requested a review from Ch3LL October 12, 2020 15:50
@major0 major0 force-pushed the fix/stop_checking_unused_boto_versions branch from 0a298f8 to 38eee3f Compare October 12, 2020 15:53
@major0 major0 mentioned this pull request Oct 13, 2020
3 tasks
@major0 major0 force-pushed the fix/stop_checking_unused_boto_versions branch 2 times, most recently from 3df1e0e to 23676af Compare October 18, 2020 18:23
salt/utils/versions.py Show resolved Hide resolved
changelog/58715.md Outdated Show resolved Hide resolved
salt/utils/boto3mod.py Show resolved Hide resolved
@major0 major0 force-pushed the fix/stop_checking_unused_boto_versions branch from 23676af to 868eaeb Compare October 20, 2020 16:06
@major0 major0 closed this Oct 20, 2020
@major0 major0 reopened this Oct 20, 2020
@major0 major0 force-pushed the fix/stop_checking_unused_boto_versions branch 3 times, most recently from 08f0c56 to f73c934 Compare October 20, 2020 20:34
salt/utils/versions.py Outdated Show resolved Hide resolved
@major0 major0 force-pushed the fix/stop_checking_unused_boto_versions branch 3 times, most recently from 56a2482 to 071c44a Compare October 21, 2020 13:58
Ch3LL
Ch3LL previously approved these changes Oct 22, 2020
@Ch3LL Ch3LL requested a review from a team October 22, 2020 10:49
changelog/58715.changed Outdated Show resolved Hide resolved
waynew
waynew previously approved these changes Oct 22, 2020
Most boto/boto3 modules do not actively use both boto and boto3.  This
commit modifies the check_boto_reqs() utility function to stop assuming
a boto version and to only perform a boto check for a specific version
of boto should a boto version be specified.
A number of boto3 modules contain boto checks and imports, but do not
actually make any boto calls via the boto libraries.
@major0 major0 dismissed stale reviews from waynew and Ch3LL via 142720d October 23, 2020 11:07
@major0 major0 force-pushed the fix/stop_checking_unused_boto_versions branch from 071c44a to 142720d Compare October 23, 2020 11:07
waynew
waynew approved these changes Oct 23, 2020
@Ch3LL
Copy link
Contributor

Ch3LL commented Mar 30, 2021

Apologies on the late follow up here, but it looks like there is some merge conflicts. Can I help with anything?

@Ch3LL
Copy link
Contributor

Ch3LL commented Oct 4, 2022

@major0 are you willing to come back to this PR and fix the merge conflicts?

@Ch3LL
Copy link
Contributor

Ch3LL commented Oct 31, 2022

Closing due to inactivity. Please let me know if I need to re-open or please open against master with the merge conflict resolved.

@Ch3LL Ch3LL closed this Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants