Ensure RedisIntegration is disabled, unless redis is installed

[szokeasaurusrex]

This PR fixes a bug where the RedisIntegration was showing up in the list of enabled integrations even when redis was not installed, and the integration should have been disabled.

Fixes #1734

[szokeasaurusrex]

I was unsure about in which file this test should be placed. Is it ok to keep here, or should it go somewhere else?

[sentrivana]

Maybe test_basics.py would be better? It already has some tests for integration enabling.

test_api.py is more for testing the global API (configure_scope, start_transaction, etc.; the stuff we surface via

sentry-python/sentry_sdk/__init__.py

Lines 13 to 43 in cb7299a

	__all__ = [ # noqa
	"Hub",
	"Scope",
	"Client",
	"Transport",
	"HttpTransport",
	"init",
	"integrations",
	# From sentry_sdk.api
	"capture_event",
	"capture_message",
	"capture_exception",
	"add_breadcrumb",
	"configure_scope",
	"push_scope",
	"flush",
	"last_event_id",
	"start_span",
	"start_transaction",
	"set_tag",
	"set_context",
	"set_extra",
	"set_user",
	"set_level",
	"set_measurement",
	"get_current_span",
	"get_traceparent",
	"get_baggage",
	"continue_trace",
	"trace",
	]

).

[sentrivana]

@szokeasaurusrex If I understood the issue correctly, the underlying problem is that in the Redis integration we raise DidNotEnable in setup_once rather than on module level. If that's the case, would moving the check

sentry-python/sentry_sdk/integrations/redis/__init__.py

Lines 275 to 278 in cb7299a

	try:
	from redis import StrictRedis, client
	except ImportError:
	raise DidNotEnable("Redis client not installed")

to the top level imports, just like we do in other integrations, solve the issue?

[szokeasaurusrex]

@sentrivana Apparently it is also a more general issue. Integrations which conditionally raise DidNotEnable in the setup_once function are also affected, according to the original issue. From the issue:

However, other automatically enabled integrations that raise DidNotEnable in setup_once for other reasons also encounter this issue, e.g. aiohttp with v3.3

So, raising DidNotEnable when importing the RedisIntegration would probably fix the issue specifically for Redis, but we would still probably need this change to fix the more general problem.

[sentrivana]

@szokeasaurusrex Gotcha, I see now. We still raise DidNotEnable in setup_once in virtually all integrations when we succeed in importing the package, but it's somehow incompatible (unsupported version, failure to parse the version altogether, etc.).

I'll give this a proper review tomorrow 👍🏻

[szokeasaurusrex]

We still raise DidNotEnable in setup_once in virtually all integrations when we succeed in importing the package, but it's somehow incompatible (unsupported version, failure to parse the version altogether, etc.).

Yes, and this is precisely the reason why the change here is needed

[sentrivana]

Thanks for fixing this!

[szokeasaurusrex]

@sentrivana Do you think this new file is an appropriate place for this test?

[sentrivana]

Why not just put it in tests/test_basics.py since we already have some integration tests in there?

Or maybe we could rename this new file from test_init.py to something more generic like test_common.py and also move other integrations-related tests from test_basics.py here.

[szokeasaurusrex]

Okay, I'll put it in test_basics.py for now. Maybe in the future we could split out into a separate file, but I think moving everything over would be out of scope for this issue

[szokeasaurusrex]

@sentrivana I added a test to check that multiple setup_integrations calls work as expected. I already validated that the test fails on b039d84. Would appreciate if you could quickly review the new test.

[Harmon758]

Thanks for the fix! and for ensuring it covers everything I detailed in the issue 👍