Do our best to ensure we don't add the same handler more than once.

[mikelambert]

Previously, constructing a new GCM(debug=True) twice would result in two debug handlers being added.

[tgwizard]

If you provide your own handler this won't work, right? If you provide a new handler the second time you call the function the old handler will still be there, which you might not want.

Perhaps the current handler, regardless if provided by the client or created here, could be stored, and then there could be a disable_logging function which just removes the handler appropriately. Either we could always disable logging before enabling it, or there could be a flag reset_logging=True in enable_logging which would call disable_logging before proceeding (if true). Or the client would have to call disable_logging before enabling logging the second time.

[mikelambert]

That's why I call removeHandler, so that if you provide the same handler twice, it won't be double-added. If you call enable_logging with two separate handlers, it would make sense to me that both handlers would be added.

Though yes, this API wasn't well-designed, especially in combination with a GCM(debug=True) case that leads one to think logging is isolated to the new GCM (when in fact, it is global). I did extra work to improve enable_logging() as well, for those who might call it with a custom handler.

But if you have a preference for me to implement a disable_logging/reset_logging implementation instead, to get this submitted, I can do that instead.

[alibitek]

@mikelambert I added the logging API only for python-gcm internal development purposes, to make it easier to see what's going on under the hood.
I haven't considered the case where more than one GCM object is constructed with debug=True, as I don't see a reason to have more than one object, except maybe when passing a requests.Session object for connection reuse , which will make the code thread unsafe and having a deep copy/clone might help depending on how the client code is structured/designed.

Thanks for making this pull request, I think it is fine with the changes you've made, but as per CONTRIBUTING.md can you please do the following two things:

1. add at least one test that covers the behaviour with two GCM objects and ensures that a single handler is added
2. make sure your changes are made to the develop branch

[alibitek]

@mikelambert I've updated my explanation above. Can you also provide a use case where you needed to create more than one GCM object?

[mikelambert]

I'm calling GCM from AppEngine, and so creating and using a GCM object as part of fulfilling the request. I didn't see anything about thread-safety in the code, and I generally avoid using complex shared module-level objects due to concurrency fears.

But it sounds like from your above comment that these objects are thread-safe, and I can just re-use the GCM object. If so, that works for me.

Though I might suggest comments/docstrings in the code/readme discussing the threadsafe nature of these objects, and the internal-only-ness of the debug= parameter. (I needed it when trying to diagnose why my GCM calls weren't working, but that was due to requests-on-appengine being broken, which I've got another set of pull requests out to fix. You're right that in the steady-state case, I don't actually need GCM debugging at all.)

[alibitek]

@mikelambert Thanks for the PR.

I took the commit you've made and added it to the develop branch myself and as the code coverage has not decreased it is okay even without a test.

5d9c702