Fix #624 by removing unnecessary boto3 version peg

[hexDoor]

resolves #624

Problem

The boto3 dependency is as of writing, pegged 7 major versions behind the latest boto3 release.
The only context where boto3 is referenced within the dbt-redshift repository is within a unit test which raises the question on whether the boto3 version peg is even necessary since it is already a transitive dependency of redshift-connector.
At the current time, this version peg frustrates compatibility of dbt-redshift with the latest awscli, boto3 and botocore versions as well as other unrelated dbt plugins.

Solution

Remove boto3 as a direct dependency for dbt-redshift

Checklist

• I have read the contributing guide and understand what's expected of me
• I have run this code in development and it appears to resolve the stated issue
• This PR includes tests, or tests are not required/relevant for this PR
• This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX

[cla-bot]

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR.

CLA has not been signed by users: @hexDoor

[hexDoor]

CLA has now been signed.

[mikealfare]

@cla-bot check

[cla-bot]

The cla-bot has been summoned, and re-checked this pull request!

[mikealfare]

@hexDoor Do you mind adding a changelog entry (changie new)?

@dbt-labs/core-adapters How do folks feel about this? I would normally say to at least keep the dependency in the requirements, unpinned, if we use it directly. But given the limited usage and the fact that we're likely going to have redshift-connector here for the foreseeable future, I think it's alright to remove it entirely. This was probably a holdover from before we started using redshift-connector and should have been removed at that point.

[VersusFacit]

I'm pretty anxious about unintended consequences here. Sure, we aren't using this directly but what about this being decided by other packages? Indeed, we've got failures here. We need to ensure this change is self-contained before moving forward.

[hexDoor]

Sure, we aren't using this directly but what about this being update by other packages?

@VersusFacit Could I confirm what you mean by this?
If we're talking about how other packages are using dbt-redshift, I would be very concerned for the dependency management of those packages if they were using boto3 in their package without declaring a direct dependency.
With that in mind, I would say with reasonable certainty that removing this dependency is not going to affect end users who are following best practices.

As for the test failure (tests/functional/adapter/test_basic.py::TestDocsGenerateRedshift::test_run_and_generate), I believe it is unrelated to this change and might be resolved with a rerun.

17:50:14  Encountered an error while generating catalog: Database Error
  could not open relation with OID 22039866
17:50:14  dbt encountered 1 failure while writing the catalog
17:50:14  Catalog written to /tmp/pytest-of-runner/pytest-0/popen-gw3/project3/target/catalog.json

[VersusFacit]

If we're talking about how other packages are using dbt-redshift, I would be very concerned for the dependency management of those packages if they were using boto3 in their package without declaring a direct dependency.

Having worked in this space for a few years now, I take absolutely nothing for granted as to the dependency management of Python packages. We've had a lot of incidents over the years where things move out from underneath us and break unceremoniously. Several version pins are there frankly to protect us at times. Hence, my desire for caution and verification.

I'll give things a rerun just in case you're right. I'd love to remove dead code if this is indeed superfluous

[hexDoor]

Having worked in this space for a few years now, I take absolutely nothing for granted as to the dependency management of Python packages. We've had a lot of incidents over the years where things move out from underneath us and break unceremoniously. Several version pins are main frankly to protect us at times. Hence, my desire for caution and verification.

I see. I would love to avoid causing an incident myself (especially on the public stage) 😆
However, would love to see some RCA's or discussions on some of these incidents out of interest.

I'll give things a rerun just in case you're right.

I don't think the Adapter Integration Test workflow got re-run as I can't see a new entry in the list 😞

[mikealfare]

As for the test failure (tests/functional/adapter/test_basic.py::TestDocsGenerateRedshift::test_run_and_generate), I believe it is unrelated to this change and might be resolved with a rerun.

Just to remove this from the discourse, this test is a known flaky test. It fails routinely, necessitating a re-run. It is on our backlog to look into and resolve. It is unrelated to this change.

[VersusFacit]

@hexDoor

We talked about this internally a little bit. We feel comfortable with merging this PR. Before we do so though, can we get you to do one more commit? It'd be great to do the following. Transparently, it's to deal with flaky tests and to illuminate references to this package entirely where they are unneeded.

• remove each @mock.patch("boto3.Session", Mock()) found in test/unit/test_redshift_adapter.py, that's like 6 lines I think
• remove tests/functional/adapter/test_basic.py::TestDocsGenerateRedshift::test_run_and_generate to get your PR consistently green

That will get us out of stale test hell and resolve this issue at once :D

[VersusFacit]

Hooray, we're green. Thanks for doing us a solid @hexDoor and for being so responsive today ❤️

Happy to see this merged. I'm always a big fan of cleanup.