Skip to content
This repository was archived by the owner on Oct 23, 2023. It is now read-only.

By default don't import 3rd-party Python modules, only stdlib ones. … #857

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

By default don't import 3rd-party Python modules, only stdlib ones. … #857

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
096245d
By default don't import 3rd-party Python modules, only stdlib ones. …
Sep 13, 2016
7a6a6a7
Add note about non-stdlib transports no longer being imported by default
Sep 14, 2016
f339226
Remove commented out code
Sep 14, 2016
55b8dad
Fix tests
Sep 14, 2016
6550d8b
Don't enable compatibility registration of all transports in tests.
Sep 23, 2016
049b98b
In CHANGES, recommend to use the `transport` kwarg of raven.Client()
Sep 23, 2016
11c58cd
tornado test can now use http:// as dsn instead of tornado+http://
gjcarneiro Sep 23, 2016
6125748
Fix gevent transport unit test
Sep 28, 2016
5151958
Merge remote-tracking branch 'origin/master' into dont_import_the_world
May 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions CHANGES
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,37 @@ Version 5.27.0

* Updated CA bundle.

* Raven transports that have dependencies outside Python stdlib are no longer
imported by default, and must now be explicitly imported by the application.
The transports no longer imported by default are:
- raven.transport.eventlet.EventletHTTPTransport
- raven.transport.gevent.GeventedHTTPTransport
- raven.transport.requests.RequestsHTTPTransport
- raven.transport.threaded_requests.ThreadedRequestsHTTPTransport
- raven.transport.twisted.TwistedHTTPTransport
- raven.transport.tornado.TornadoHTTPTransport

If you really need to have those transports available, you need to explicitly
import them and register them. For full compatibility:
```
from raven.transport.eventlet import EventletHTTPTransport
from raven.transport.gevent import GeventedHTTPTransport
from raven.transport.requests import RequestsHTTPTransport
from raven.transport.twisted import TwistedHTTPTransport
from raven.transport.tornado import TornadoHTTPTransport
from raven.transport.threaded_requests import ThreadedRequestsHTTPTransport
from raven.base import Client

for transport in [EventletHTTPTransport,
Copy link
Member

Choose a reason for hiding this comment

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

of note we dont actually need to do this since you can pass transport to the Client() itself. It was purely for backwards compatibility for a period of time.

Copy link
Author

Choose a reason for hiding this comment

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

But the problem is that a typical Django setup just adds raven.contrib.django.raven_compat to the list of INSTALLED_APPS. If they also have a weird DSN configured, they would need to register the transports in the registry for it to work, as the Django Sentry app creates its own Client() and I honestly have no idea how to customize creation of this Client()...

Copy link
Member

Choose a reason for hiding this comment

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

You can actually just pass it into SENTRY_CONFIG via 'transport' (same as all of the other config options).

Copy link
Author

Choose a reason for hiding this comment

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

Ah, OK then. I guess I can update the documentation to reflect this. Seems cleaner the way you suggest.

GeventedHTTPTransport,
RequestsHTTPTransport,
ThreadedRequestsHTTPTransport,
TwistedHTTPTransport,
TornadoHTTPTransport]:
Client._registry.register_transport(transport)

```

Version 5.26.0
--------------

Expand Down
18 changes: 18 additions & 0 deletions conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@
import os.path
import sys

from raven.transport.eventlet import EventletHTTPTransport
from raven.transport.gevent import GeventedHTTPTransport
from raven.transport.requests import RequestsHTTPTransport
from raven.transport.twisted import TwistedHTTPTransport
from raven.transport.tornado import TornadoHTTPTransport
from raven.transport.threaded_requests import ThreadedRequestsHTTPTransport
from raven.base import Client


collect_ignore = []
if sys.version_info[0] > 2:
Expand Down Expand Up @@ -82,3 +90,13 @@ def pytest_configure(config):
}],
ALLOWED_HOSTS=['*'],
)



for transport in [EventletHTTPTransport,
Copy link
Member

Choose a reason for hiding this comment

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

assuming things break without this?

Copy link
Author

Choose a reason for hiding this comment

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

Sadly yes. The reason is that all tests build clients based on DSN, so a few test cases fail. And I guess using the DSN and registry is part of the things we test, so we can't remove it?

_________________________________________________________________________________________________ ThreadedTransportTest.test_does_send __________________________________________________________________________________________________
InvalidDsn: Unsupported Sentry DSN scheme: threaded+requests+http (threaded+requests+http://some_username:some_password@localhost:8143/1)
__________________________________________________________________________________________ ThreadedTransportTest.test_shutdown_waits_for_send ___________________________________________________________________________________________
InvalidDsn: Unsupported Sentry DSN scheme: threaded+requests+http (threaded+requests+http://some_username:some_password@localhost:8143/1)
_________________________________________________________________________________________________ RequestsTransportTest.test_does_send __________________________________________________________________________________________________
InvalidDsn: Unsupported Sentry DSN scheme: requests+http (requests+http://some_username:some_password@localhost:8143/1)
________________________________________________________________________________ TornadoTransportTests.test__sending_successfully_calls_success_callback ________________________________________________________________________________
InvalidDsn: Unsupported Sentry DSN scheme: tornado+http (tornado+http://uver:pass@localhost:46754/1)
__________________________________________________________________________________ TornadoTransportTests.test__sending_with_error_calls_error_callback __________________________________________________________________________________
InvalidDsn: Unsupported Sentry DSN scheme: tornado+http (tornado+http://uver:pass@localhost:46754/1)
____________________________________________________________________________________________________ TornadoTransportTests.test_send ____________________________________________________________________________________________________
InvalidDsn: Unsupported Sentry DSN scheme: tornado+https (tornado+https://user:pass@host:1234/1?timeout=1&verify_ssl=1&ca_certs=/some/path/somefile)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah we'd have to update these tests to pass it in directly, but I'd be ok with that in order to remove the legacy compatibility.

Copy link
Author

Choose a reason for hiding this comment

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

OK. Will probably have to wait a few days before I commit more changes, as I'll be travelling today (to PyCon UK) and I don't have a laptop.

GeventedHTTPTransport,
RequestsHTTPTransport,
ThreadedRequestsHTTPTransport,
TwistedHTTPTransport,
TornadoHTTPTransport]:
Client._registry.register_transport(transport)
8 changes: 0 additions & 8 deletions raven/transport/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,4 @@
from __future__ import absolute_import

from raven.transport.base import * # NOQA
from raven.transport.eventlet import * # NOQA
from raven.transport.exceptions import * # NOQA
from raven.transport.gevent import * # NOQA
from raven.transport.http import * # NOQA
from raven.transport.requests import * # NOQA
from raven.transport.registry import * # NOQA
from raven.transport.twisted import * # NOQA
from raven.transport.threaded import * # NOQA
from raven.transport.tornado import * # NOQA
13 changes: 0 additions & 13 deletions raven/transport/registry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,9 @@
"""
from __future__ import absolute_import

# TODO(dcramer): we really should need to import all of these by default
from raven.transport.eventlet import EventletHTTPTransport
from raven.transport.exceptions import DuplicateScheme
from raven.transport.http import HTTPTransport
from raven.transport.gevent import GeventedHTTPTransport
from raven.transport.requests import RequestsHTTPTransport
from raven.transport.threaded import ThreadedHTTPTransport
from raven.transport.threaded_requests import ThreadedRequestsHTTPTransport
from raven.transport.twisted import TwistedHTTPTransport
from raven.transport.tornado import TornadoHTTPTransport
from raven.utils import urlparse


Expand Down Expand Up @@ -74,10 +67,4 @@ def compute_scope(self, url, scope):
default_transports = [
HTTPTransport,
ThreadedHTTPTransport,
GeventedHTTPTransport,
TwistedHTTPTransport,
RequestsHTTPTransport,
ThreadedRequestsHTTPTransport,
TornadoHTTPTransport,
EventletHTTPTransport,
]
2 changes: 1 addition & 1 deletion raven/transport/threaded_requests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from __future__ import absolute_import

from raven.transport.base import AsyncTransport
from raven.transport import RequestsHTTPTransport
from raven.transport.requests import RequestsHTTPTransport
from raven.transport.threaded import AsyncWorker


Expand Down
2 changes: 1 addition & 1 deletion tests/base/tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

from raven.base import Client, ClientState
from raven.exceptions import RateLimited
from raven.transport import AsyncTransport
from raven.transport.base import AsyncTransport
from raven.transport.http import HTTPTransport
from raven.utils.stacks import iter_stack_frames
from raven.utils.testutils import TestCase
Expand Down
2 changes: 1 addition & 1 deletion tests/contrib/django/tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
from raven.contrib.django.middleware.wsgi import Sentry
from raven.contrib.django.templatetags.raven import sentry_public_dsn
from raven.contrib.django.views import is_valid_origin
from raven.transport import HTTPTransport
from raven.transport.http import HTTPTransport
from raven.utils.serializer import transform

from django.test.client import Client as TestClient, ClientHandler as TestClientHandler
Expand Down
4 changes: 2 additions & 2 deletions tests/transport/requests/test_threaded_requests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

class DummyThreadedScheme(ThreadedRequestsHTTPTransport):
def __init__(self, *args, **kwargs):
super(ThreadedRequestsHTTPTransport, self).__init__(*args, **kwargs)
super(DummyThreadedScheme, self).__init__(*args, **kwargs)
self.events = []
self.send_delay = 0

Expand All @@ -26,7 +26,7 @@ def setUp(self):
self.url = "threaded+requests+http://some_username:some_password@localhost:8143/1"
self.client = Client(dsn=self.url)

@mock.patch('raven.transport.requests.post')
@mock.patch('requests.post')
def test_does_send(self, send):
self.client.captureMessage(message='foo')

Expand Down
2 changes: 1 addition & 1 deletion tests/transport/requests/tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def setUp(self):
dsn="requests+http://some_username:some_password@localhost:8143/1",
)

@mock.patch('raven.transport.requests.post')
@mock.patch('requests.post')
def test_does_send(self, post):
self.client.captureMessage(message='foo')
self.assertEqual(post.call_count, 1)
Expand Down
2 changes: 1 addition & 1 deletion tests/transport/threaded/tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

class DummyThreadedScheme(ThreadedHTTPTransport):
def __init__(self, *args, **kwargs):
super(ThreadedHTTPTransport, self).__init__(*args, **kwargs)
super(DummyThreadedScheme, self).__init__(*args, **kwargs)
self.events = []
self.send_delay = 0

Expand Down