LB-196: Remove duplication of RabbitMQ connection code

[ghost]

Summary

• This is a…
  • Bug fix
  • Feature addition
  • Refactoring
  • Minor / simple change (like a typo)
  • Other
• Describe this change in 1-2 sentences:

This change extracts the common RabbitMQ connection code into one function in utils.py.

Problem

• JIRA ticket: https://tickets.metabrainz.org/browse/LB-196

Solution

connect_to_rabbitmq in utils.py tries repeatedly until it's able to connect to RabbitMQ and then returns the Connection object.

Action

@paramsingh

[ghost]

I am not sure where this Travis error is coming from. It appeared in #308 as well, seemingly randomly. It failed on one commit, and when I committed again without even touching the code, it passed. The error is tarfile throwing a ReadError when import_listens_dump tries to open a particular dump_location, but I do not know what location this is referring to. It doesn't really appear to be a Travis problem since I ran ./test.sh locally just now and it failed in the same way.

[ghost]

@paramsingh Out of curiosity, I ran ./test.sh two minutes ago on my local copy of this branch, without any local directory changes, no new commits, nothing. Magically, it worked. So I thought the issue kind of resolved itself here, or at the very least I could make the Travis build pass for now. I made a really minor change to a docstring so I would have something I could run git commit --amend on. I force-pushed, and the Travis build fails, once again in the same way.

It seems to be a problem with how the Travis machine is configured or started, or something because I can't seem a pattern here.

[paramsingh]

Instead of adding an extra function to utils, creating a new ListenWriter class from which we inherit methods in BQ writer and Influx writer might be a good idea. We could then try to move other methods out to that class as well.

[paramsingh]

Just do an import listenbrainz.utils as utils here too.

[paramsingh]

The test failures seem unrelated, I'll take a look when I get the chance.

[paramsingh]

I know i said adding to the utils file was what I said in the task but the class seems like a better idea now. 🙄

[ghost]

@parasingh I have a few questions related to the task. If I create a new class ListenWriter and inherit from it in BQ listener and Influx listener, do you want the whole util.connect_to_rabbitmq method to go there?

In this case, what should be done for webserver/rabbitmq_connection.py, which only has a method called init_rabbitmq_connection?

Do you want to move any other methods to this new base class?

[ghost]

Made a few basic changes - a new ListenWriter class, with some functionality I was able to identify as common, is the base class of BigQueryWriter and InfluxWriter classes. There's some more stuff left, like the initial code in the start methods, but I wasn't able to extract any semantic meaning out of those sections myself - I would love some suggestions on it.

[paramsingh]

Consistency here between " and ' would be appreciated.

[paramsingh]

I prefer ' :)

[paramsingh]

Mind taking a look at why the integration tests fail?

The tarfile test is unrelated, but these failures seem to be caused due to the changes made in the code here.

============================= test session starts ==============================

platform linux -- Python 3.6.1, pytest-2.9.1, py-1.5.2, pluggy-0.3.1

rootdir: /code/listenbrainz, inifile: pytest.ini

plugins: cov-2.2.1

collected 39 items 

listenbrainz/tests/integration/test_api.py FF.................

listenbrainz/tests/integration/test_api_compat_deprecated.py .........F....

listenbrainz/tests/integration/test_influx_writer.py FFFFF

listenbrainz/tests/integration/test_profile_views.py F

=================================== FAILURES ===================================
``

[ghost]

@paramsingh Fixed. Actually I didn't realize integration_test.sh existed 😥 But anywho, the failures were caused by the ListenWriter not being imported properly, and me not changing config to self.config in InfluxWriter. I did it in BQWriter, but not Influx 🤦‍♀️ .

EDIT: YES! Travis is a ✅

Fixed quote problem as well!