praekeltfoundation / vumi

Messaging engine for the delivery of SMS, Star Menu and chat messages to diverse audiences in emerging markets and beyond.
BSD 3-Clause "New" or "Revised" License
422 stars 131 forks source link

Force SMPP data_coding keys to be ints #1045

Closed imsickofmaps closed 7 years ago

imsickofmaps commented 8 years ago

This is needed because some dynamic config producers are not able to store keys as ints (like Junebug)

imsickofmaps commented 8 years ago

@jerith @smn @rudigiesler @justinvdm can someone review this and release if acceptable?

Apologies for the @ bomb - I discussed this fix with @hodgestar and it's a known issue discussed with @rudigiesler back in May but the fix on Junebug would be quite complex and this is simple.

hodgestar commented 8 years ago

We might want to raise a ConfigError with a suitable message rather than raising a TypeError or ValueError.

smn commented 8 years ago

A test would be nice! It's in your fork and so I cannot contribute it easily to your repo.

imsickofmaps commented 8 years ago

I wasn't sure how you would do tests for this as you haven't got anything testing config that I could see, without duplicating a large chunk of test setup stuff. I did it in my fork because I don't have access to this repo that I know of. Happy to add you to mine if it's quicker to show rather than tell on the adding tests issue?

smn commented 8 years ago

@imsickofmaps I would expect the following test to fail for you:

from twisted.internet.task import Clock

from vumi.errors import ConfigError
from vumi.tests.helpers import VumiTestCase
from vumi.transports.tests.helpers import TransportHelper
from vumi.transports.smpp.smpp_transport import SmppTransceiverTransport
from vumi.transports.smpp.tests.fake_smsc import FakeSMSC

class DefaultProcessorTestCase(VumiTestCase):

    def setUp(self):
        self.clock = Clock()
        self.fake_smsc = FakeSMSC()
        self.tx_helper = self.add_helper(
            TransportHelper(SmppTransceiverTransport))

    def test_data_coding_strings(self):
        cfg = {
            'system_id': 'foo',
            'password': 'bar',
            'transport_name': self.tx_helper.transport_name,
            'twisted_endpoint': self.fake_smsc.endpoint,
            'deliver_short_message_processor_config': {
                'data_coding_overrides': {
                    'not-an-int': 'utf-8',
                }
            },
        }
        return self.assertFailure(
            self.tx_helper.get_transport(cfg), ConfigError)
rudigiesler commented 7 years ago

Solved by https://github.com/praekelt/vumi/issues/1063