Skip to content

Commit

Permalink
Update warning when invalid delivery is set
Browse files Browse the repository at this point in the history
  • Loading branch information
imjoehaines committed Feb 26, 2024
1 parent 294dc20 commit 77e1174
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 0 deletions.
19 changes: 19 additions & 0 deletions bugsnag/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,25 @@ def delivery(self):
def delivery(self, value):
if hasattr(value, 'deliver') and callable(value.deliver):
self._delivery = value

# deliver_sessions is _technically_ optional in that if you disable
# session tracking it will never be called
# this should be made mandatory in the next major release
if (
hasattr(value, 'deliver_sessions') and
callable(value.deliver_sessions)
):
parameter_names = value.deliver_sessions.__code__.co_varnames

This comment has been minimized.

Copy link
@surkova

surkova May 17, 2024

This line broke our mocking of bugsnag for dev:

from unittest.mock import Mock

import bugsnag
from bugsnag.delivery import Delivery
import pytest

@pytest.fixture(scope="session", autouse=True)
def mock_bugsnag_integration():
    """Mock full integration with bugsnag once"""
    mock_delivery = Mock(spec=Delivery)
    bugsnag.configure(delivery=mock_delivery)

    yield mock_delivery

Since __code__ is a magic attribute, you cannot mock it out.

This comment has been minimized.

Copy link
@imjoehaines

imjoehaines May 17, 2024

Author Contributor

Thanks for the report, @surkova. I'll add a fix for this in the next release

This comment has been minimized.

Copy link
@imjoehaines

imjoehaines May 17, 2024

Author Contributor

Fixed in #387, which should go out next week

This comment has been minimized.

Copy link
@surkova

surkova May 20, 2024

Thank you!

This comment has been minimized.

Copy link
@imjoehaines

imjoehaines May 22, 2024

Author Contributor

I released this in v4.7.1 this morning 🙂

Sorry for the inconvenience!


if 'options' not in parameter_names:
warnings.warn(
'delivery.deliver_sessions should accept an ' +
'"options" parameter to allow for synchronous ' +
'delivery, sessions may be lost when the process ' +
'exits',
DeprecationWarning
)

else:
message = ('delivery should implement Delivery interface, got ' +
'{0}. This will be an error in a future release.')
Expand Down
21 changes: 21 additions & 0 deletions tests/test_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,13 @@ class BadDelivery(object):
def deliv(self, *args, **kwargs):
pass

class OkDelivery:
def deliver(self, *args, **kwargs):
pass

def deliver_sessions(self, config, payload):
pass

class GoodDelivery(object):
def deliver(self, *args, **kwargs):
pass
Expand All @@ -213,6 +220,20 @@ def deliver(self, *args, **kwargs):
assert len(record) == 1
assert c.delivery == good

with pytest.warns(DeprecationWarning) as record:
ok = OkDelivery()

c.configure(delivery=ok)

assert len(record) == 1
assert str(record[0].message) == (
'delivery.deliver_sessions should accept an "options" ' +
'parameter to allow for synchronous delivery, sessions ' +
'may be lost when the process exits'
)

assert c.delivery == ok

def test_validate_hostname(self):
c = Configuration()
with pytest.warns(RuntimeWarning) as record:
Expand Down

0 comments on commit 77e1174

Please sign in to comment.