Adds ArbitraryContextAdapter.

[kurtwheeler]

I have created a new adapter which works well with the rest of the daiquiri system. This adapter allows a single logger to be passed arbitrary parameters which will then be formatted and included in the log message in place of the %(context)s match. I plan on using this in my own project, but it seems like it could be generally useful and fits within the scope of the daiquiri project.

Here is an example of how it is used:

>>> name = "my_module"
>>> logger = daiquiri.getLogger(name,
                                adapter_class=daiquiri.ArbitraryContextAdapter,
                                level=logging.INFO)
>>> handler = logging.StreamHandler(sys.stdout)
>>> format_string = "%(asctime)s %(name)s %(levelname)s%(context)s: %(message)"
>>> handler.setFormatter(daiquiri.formatter.ColorFormatter(fmt=format_string))
>>> logger.logger.addHandler(handler)
>>> logger.info("A message.", test1="a", test2="b")
2017-08-18 17:10:20,212 my_module INFO [test1: a] [test2: b]: A message.
>>> logger.info("Another message.", test1="a", test2="b", test3="c")
2017-08-18 17:10:20,212 my_module INFO [test1: a] [test2: b] [test3: c]: A message.

Currently there are a couple of small quirks with this PR as is. They are acceptable for my project, but I would be happy to clean them up if this is functionality is something you would in fact consider for this library. These quirks are:

• The context string is preceded by an extra space character.
• The parameter template is not configurable. I believe I could overwrite the constructor for the ArbitraryContextAdapter to accept a parameter template instead of always using [{0}: {1}].

[kurtwheeler]

OK @jd I have implemented the new API for this functionality. It seems to be working well and matching the way I said it would work based on some functional tests I ran.

The one thing I am aware of that could potentially be improved is the way the "extras" string is built. Currently if there are zero "extras" then the string will be '', if there's one it will be '[a: b]', and if there's two it will be '[a: b] [c: d]'. This seems like a good default, except if try to build your format string around like '%(level)s %(extras)s: %(message)s', then it can end up looking funny if there aren't any extras. (E.g. INFO : my message, notice the extra space before the colon.)

If we had it so that the extras was always preceded by a space then you make your format string '%(level)s%(extras)s: %(message)s' so that if you had extras the message would be INFO [a: b]: my message and if you didn't it'd be INFO: my message. I don't know how big of a deal this is, but I'm also not sure which is more desirable.

[jd]

One last thing: it should be easy to add some unit tests and that'd be really great so we're sure not to break anything.

[kurtwheeler]

Alright, I've pushed my changes which I believe address all of your concerns and feedback. The tests I've added are passing, but Travis is failing for some reason that I do not understand. Do you think you could take a look and see if you know what's going on?

[kurtwheeler]

BTW, the tests were somewhat hard to figure out. I think it would make this project easier to contribute to if you had a section of the README dedicated to development. It'd be helpful if it contained things like how to get setup and how to run the tests. I created a virtualenv for this project and basically just installed packages one by one because there's no requirements.txt and I've never used tox before, so I have no clue what it even does.

[jd]

The tests failure in Python 3.6 are due to pep8, because your import are not in the right order.

./daiquiri/tests/test_formatter.py:3:1: H306  imports not in alphabetical order (testtools, six)
import six
^
./daiquiri/tests/test_formatter.py:4:1: H306  imports not in alphabetical order (six, daiquiri)
import daiquiri
^

The tests failure in Python 2.7 looks legit thought.

I've never used tox before, so I have no clue what it even does.

Yeah I guess that's the problem. Though tox is super standard nowadays, I can't think of a project not using it. :)

Just type tox and it should work ;)

[kurtwheeler]

Yeah I guess that's the problem. Though tox is super standard nowadays, I can't think of a project not using it. :)

I'm relatively new to the python ecosystem, so I don't know all the tools (and probably never will TBH). I also don't work on a library so I don't have to worry about supporting multiple versions of python.

However my suggestion to include a Contributing or Developing section of the README is also super standard, I can't think of any projects that don't have one. You never know what the background of your contributors will be, so why not make it easy for them to figure out the build/test tools used in your project?

[jd]

However my suggestion to include a Contributing or Developing section of the README is also super standard, I can't think of any projects that don't have one. You never know what the background of your contributors will be, so why not make it easy for them to figure out the build/test tools used in your project?

Oh don't misread me. I'm not against adding it, like, at all. I'm just super lazy. ;)

[kurtwheeler]

Hey @jd I've made additional changes. Looks like everything is good except for Python 2.7 inheritance. KeywordArgumentAdapter inherits from logging.LoggerAdapter which has a setLevel method. However it doesn't seem to be working right, but only in Python 2.7. Do you know how to resolve this issue?

[jd]

@kurtwheeler https://github.com/jd/daiquiri/commit/dc09d57013ac2bd9b96c5727c55aa0b48a258374 should fix your problem. Just rebase your branch (and squash your commits while you're at it) and that should work!

[jd]

Thanks @kurtwheeler! I've merged your code in a single final commit.

[kurtwheeler]

Cool, thanks @jd! I was going to do the squash merge but I've been AFK for a few days. I'm glad this worked out.

[kurtwheeler]

@jd I'm not exactly sure how you merged this PR in but it looks like you missed some commits, including the one where I added all the tests.

[kurtwheeler]

P.S. I just realized that [level: 20] will show up in the extras string. I've pushed a commit that removes it to my branch in my fork.

[jd]

@kurtwheeler I had to merge manually because of the messy history :D I think I just missed the test file, so I've just repushed a commit to master with it! Thanks for noticing!

[jd]

@kurtwheeler Where the [level: 20] comes from? I don't see anything about that in the tests so I wonder what adds it.

[kurtwheeler]

Ah, the [level: 20] thing came from me using the library incorrectly, please disregard that :)