From 767f45805f79a68e9c3ef386dfc3dd6c2cc92422 Mon Sep 17 00:00:00 2001 From: Ryan Mclean Date: Thu, 8 Aug 2019 13:42:26 -0400 Subject: [PATCH 1/4] Fix bug in publishing confirmation - Fix issue opening a channel is not checking if the conn is still open - Fix issue with publishing confirmation bookkeeping not reset when channel is reopened - Add bootstrap and docker-compose instead of using local rabbitmq - Update CI to run bootstrap before tests --- .travis.yml | 5 ++++- bootstrap.sh | 24 ++++++++++++++++++++++++ docker-compose.yml | 7 +++++++ docs/history.rst | 5 +++++ setup.cfg | 3 +++ sprockets/mixins/amqp/__init__.py | 8 +++++++- tests/__init__.py | 13 +++++++++++++ tests/client_tests.py | 21 +++++++++++++++++++++ 8 files changed, 84 insertions(+), 2 deletions(-) create mode 100755 bootstrap.sh create mode 100644 docker-compose.yml diff --git a/.travis.yml b/.travis.yml index 2ddbc4b..bf33827 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,10 +4,13 @@ dist: trusty python: - 3.5 - 3.6 +- 3.7 services: -- rabbitmq +- docker install: - pip install -r requires/testing.txt -r docs/requirements.txt codecov +before_script: +- ./bootstrap.sh script: - nosetests after_success: diff --git a/bootstrap.sh b/bootstrap.sh new file mode 100755 index 0000000..3a58a4a --- /dev/null +++ b/bootstrap.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env sh +set -e + +TEST_HOST="${TEST_HOST:-127.0.0.1}" + +get_exposed_port() { + docker-compose port "$@" | cut -d: -f2 +} + +rm -rf build && mkdir build + +docker-compose down --timeout 0 --volumes --remove-orphans +docker-compose pull -q +docker-compose up -d + +echo "Environment variables (build/test-environment):" +tee build/test-environment << EOF +export AMQP_EXCHANGE=amq.topic +export AMQP_URL=amqp://guest:guest@$TEST_HOST:$(get_exposed_port rabbitmq 5672)/%2f +export ASYNC_TEST_TIMEOUT=10 +export RABBIMQ_URL=http://guest:guest@$TEST_HOST:$(get_exposed_port rabbitmq 15672) +EOF + +echo 'Bootstrap complete' diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 0000000..864afce --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,7 @@ +version: "2.4" +services: + rabbitmq: + image: rabbitmq:management-alpine + ports: + - 5672:5672 + - 15672:15672 diff --git a/docs/history.rst b/docs/history.rst index cd6c46b..9b4d56f 100644 --- a/docs/history.rst +++ b/docs/history.rst @@ -1,6 +1,11 @@ Version History =============== +`2.2.0`_ Aug 8, 2019 +--------------------- +- Fix issue opening a channel is not checking if the conn is still open +- Fix issue with publishing confirmation bookkeeping not reset when channel is reopened + `2.1.5`_ July 3, 2019 --------------------- - Remove official support for python versions less than 3.5 diff --git a/setup.cfg b/setup.cfg index 7456d96..e507bd3 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,9 @@ [bdist_wheel] universal = 1 +[coverage:report] +show_missing = 1 + [nosetests] cover-branches = 1 cover-erase = 1 diff --git a/sprockets/mixins/amqp/__init__.py b/sprockets/mixins/amqp/__init__.py index a764f73..57d7e08 100644 --- a/sprockets/mixins/amqp/__init__.py +++ b/sprockets/mixins/amqp/__init__.py @@ -407,6 +407,9 @@ def _open_channel(self): """ LOGGER.debug('Creating a new channel') + if not self.connection.is_open: + LOGGER.info('Channel connection is closed, waiting for reconnect') + return return self.connection.channel(self.on_channel_open) def _reconnect(self): @@ -548,6 +551,8 @@ def on_channel_open(self, channel): LOGGER.debug('Channel opened') self.channel = channel if self.publisher_confirmations: + self.messages.clear() + self.message_number = 0 self.channel.confirm_delivery(self.on_delivery_confirmation) self.channel.add_on_close_callback(self.on_channel_closed) self.channel.add_on_flow_callback(self.on_channel_flow) @@ -571,9 +576,10 @@ def on_channel_closed(self, channel, reply_code, reply_text): :param str reply_text: The text reason the channel was closed """ + LOGGER.debug('Channel is closed') for future in self.messages.values(): future.set_exception(AMQPException(reply_code, reply_text)) - self.messages = {} + if self.closing: LOGGER.debug('Channel %s was intentionally closed (%s) %s', channel, reply_code, reply_text) diff --git a/tests/__init__.py b/tests/__init__.py index e69de29..7bb5563 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -0,0 +1,13 @@ +import os + + +def setup_module(): + try: + with open('build/test-environment') as f: + for line in f: + if line.startswith('export '): + line = line[7:] + name, _, value = line.strip().partition('=') + os.environ[name] = value + except IOError: + pass diff --git a/tests/client_tests.py b/tests/client_tests.py index 2c223ec..902bc29 100644 --- a/tests/client_tests.py +++ b/tests/client_tests.py @@ -4,6 +4,7 @@ from pika import exceptions, frame, spec from sprockets.mixins import amqp +from tornado import concurrent from . import base @@ -242,3 +243,23 @@ def test_on_connection_open_error(self): exceptions.AMQPConnectionError('200', 'Error')) reconnect.assert_called_once() self.assertTrue(self.client.closed) + + def test_open_channel_when_connection_is_opened(self): + self.client.connection.is_open = True + self.client._open_channel() + self.assertEqual(self.client.connection.channel.call_count, 1) + + def test_open_channel_when_connection_is_not_opened(self): + self.client.connection.is_open = False + self.client._open_channel() + self.assertEqual(self.client.connection.channel.call_count, 0) + + def test_on_channel_open_reset_confirmation_bookkeeping(self): + self.client.message_number = 2 + self.client.messages = {1: concurrent.Future(), 2: concurrent.Future()} + self.client.state = amqp.Client.STATE_CONNECTING + self.assertTrue(self.client.connecting) + self.client.on_channel_open(self.client.channel) + self.assertEqual(self.client.message_number, 0) + self.assertEqual(self.client.messages, {}) + self.assertEqual(self.client.channel.confirm_delivery.call_count, 1) From 4fc8dec9e157f9969aa6681df8932f9f033baad3 Mon Sep 17 00:00:00 2001 From: Ryan Mclean Date: Thu, 8 Aug 2019 13:53:02 -0400 Subject: [PATCH 2/4] Bump version to 2.2.0 --- docs/history.rst | 5 ++++- sprockets/mixins/amqp/__init__.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/history.rst b/docs/history.rst index 9b4d56f..1e5f5b1 100644 --- a/docs/history.rst +++ b/docs/history.rst @@ -5,6 +5,8 @@ Version History --------------------- - Fix issue opening a channel is not checking if the conn is still open - Fix issue with publishing confirmation bookkeeping not reset when channel is reopened +- Add bootstrap and docker-compose instead of using local rabbitmq +- Update CI to run bootstrap before tests `2.1.5`_ July 3, 2019 --------------------- @@ -82,7 +84,8 @@ Version History ---------------------- - Initial implementation -.. _Next Release: https://github.com/sprockets/sprockets.amqp/compare/2.1.5...HEAD +.. _Next Release: https://github.com/sprockets/sprockets.amqp/compare/2.2.0...HEAD +.. _2.2.0: https://github.com/sprockets/sprockets.amqp/compare/2.1.5...2.2.0 .. _2.1.5: https://github.com/sprockets/sprockets.amqp/compare/2.1.4...2.1.5 .. _2.1.4: https://github.com/sprockets/sprockets.amqp/compare/2.1.3...2.1.4 .. _2.1.3: https://github.com/sprockets/sprockets.amqp/compare/2.1.2...2.1.3 diff --git a/sprockets/mixins/amqp/__init__.py b/sprockets/mixins/amqp/__init__.py index 57d7e08..5005342 100644 --- a/sprockets/mixins/amqp/__init__.py +++ b/sprockets/mixins/amqp/__init__.py @@ -31,7 +31,7 @@ concurrent, ioloop, exceptions, pika = \ object(), object(), object(), object() -__version__ = '2.1.5' +__version__ = '2.2.0' LOGGER = logging.getLogger(__name__) From c12bdf5ceba5447cc21dba92402d24de27f825f1 Mon Sep 17 00:00:00 2001 From: Ryan Mclean Date: Thu, 8 Aug 2019 14:02:29 -0400 Subject: [PATCH 3/4] Fix CI --- .travis.yml | 12 +++++++----- README.rst | 5 +++-- bootstrap.sh | 3 +-- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/.travis.yml b/.travis.yml index bf33827..e92dd47 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,16 +1,18 @@ sudo: false language: python -dist: trusty +dist: xenial python: - 3.5 - 3.6 - 3.7 services: -- docker +- rabbitmq +addons: + apt: + packages: + - rabbitmq-server install: - pip install -r requires/testing.txt -r docs/requirements.txt codecov -before_script: -- ./bootstrap.sh script: - nosetests after_success: @@ -23,6 +25,6 @@ deploy: password: secure: IJVu1MUk2NtRprWkYL+prPRbWrDdSiP+L06S6xERqYnu+fy1ez8/zODazkQGKagXAAujbJK8OwyCgoMzCGDNHV3/NfFtz9dirGVAD2rXZ6AVfHjtEh31L2b2YzXEK0EnBMRsYRjsqLva6q7tfxzjMWKFria25wsd9bN8VlofNDQ= on: - python: 3.6 + python: 3.7 tags: true repo: sprockets/sprockets.mixins.amqp diff --git a/README.rst b/README.rst index 2c9b00b..bec4c80 100644 --- a/README.rst +++ b/README.rst @@ -22,7 +22,7 @@ Python Compatibility -------------------- - python 3.5 - python 3.6 -- python 3.7 (currently untested in travis-ci). +- python 3.7 Requirements ------------ @@ -106,11 +106,12 @@ Source Running Tests Locally --------------------- -You'll need to have python 3.7 installed, and RabbitMQ installed locally running on port 5672 of localhost. +You'll need to have python 3.7 installed. -- $ python3.7 -m venv env -- $ env/bin/activate -- (env) $ pip install -r requires/testing.txt +-- (env) $ ./bootstrap.sh -- (env) $ nosetests License diff --git a/bootstrap.sh b/bootstrap.sh index 3a58a4a..a39acec 100755 --- a/bootstrap.sh +++ b/bootstrap.sh @@ -9,7 +9,7 @@ get_exposed_port() { rm -rf build && mkdir build -docker-compose down --timeout 0 --volumes --remove-orphans +docker-compose down --volumes --remove-orphans docker-compose pull -q docker-compose up -d @@ -17,7 +17,6 @@ echo "Environment variables (build/test-environment):" tee build/test-environment << EOF export AMQP_EXCHANGE=amq.topic export AMQP_URL=amqp://guest:guest@$TEST_HOST:$(get_exposed_port rabbitmq 5672)/%2f -export ASYNC_TEST_TIMEOUT=10 export RABBIMQ_URL=http://guest:guest@$TEST_HOST:$(get_exposed_port rabbitmq 15672) EOF From 24d1697451bfd374a0bcc49f094a598f303d1178 Mon Sep 17 00:00:00 2001 From: Ryan Mclean Date: Thu, 8 Aug 2019 15:27:05 -0400 Subject: [PATCH 4/4] Address PR feedback --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index bec4c80..8ada227 100644 --- a/README.rst +++ b/README.rst @@ -106,7 +106,7 @@ Source Running Tests Locally --------------------- -You'll need to have python 3.7 installed. +You'll need to have python 3.7, Docker and Docker Compose installed. -- $ python3.7 -m venv env -- $ env/bin/activate