Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SimpleJSONRPCServer.py: fix logging #40

Closed
wants to merge 1 commit into from

Conversation

FATruden
Copy link

@FATruden FATruden commented Oct 3, 2019

server's code:

from jsonrpclib.SimpleJSONRPCServer import PooledJSONRPCServer
from jsonrpclib.threadpool import ThreadPool
...
...
pool = ThreadPool(max_threads=50, min_threads=10)
pool.start()
server = PooledJSONRPCServer((IP, PORT), thread_pool=pool)

# All rigistred methods
server.register_function(foo)
...

When i call (on client side) unregistered method, i see in server logs:

Oct 03 12:04:52 dc01.dc.int.c2.croc.ru python[22924]: No handlers could be found for logger "jsonrpclib.SimpleJSONRPCServer"
Oct 03 12:04:52 dc01.dc.int.c2.croc.ru python[22924]: 172.20.36.102 - - [03/Oct/2019 12:04:52] "POST / HTTP/1.1" 200 -

after this fix, logging works correctly:

Oct 03 12:14:59 dc01.dc.int.c2.croc.ru python[25408]: WARNING:jsonrpclib.SimpleJSONRPCServer:Unknown method: <Fault -32601: Method test not supported.>
Oct 03 12:14:59 dc01.dc.int.c2.croc.ru python[25408]: 172.20.36.102 - - [03/Oct/2019 12:14:59] "POST / HTTP/1.1" 200 -
...
Oct 03 12:18:03 dc01.dc.int.c2.croc.ru python[25408]: WARNING:jsonrpclib.SimpleJSONRPCServer:Invalid call parameters: <Fault -32602: Invalid parameters: update() takes no arguments (1 given)>
Oct 03 12:18:03 dc01.dc.int.c2.croc.ru python[25408]: 172.20.36.102 - - [03/Oct/2019 12:18:03] "POST / HTTP/1.1" 200 -

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.4%) to 81.557% when pulling 033a33a on FATruden:logging_fix into fe9fcf2 on tcalmant:master.

@coveralls
Copy link

coveralls commented Oct 3, 2019

Coverage Status

Coverage decreased (-1.6%) to 82.346% when pulling dcdbf59 on FATruden:logging_fix into fe9fcf2 on tcalmant:master.

@tcalmant
Copy link
Owner

tcalmant commented Oct 3, 2019

Hi,
I'm not sure the logger configuration should be set by the library itself.
It is intended to use the logging configuration set by the developer using the library, at the beginning of the software.

@FATruden
Copy link
Author

FATruden commented Oct 3, 2019

Hi, i agree with you and i put logging.basicConfig() before creating server object and it works fine.
But it's a little misleading. You use logging in our code but it is not working without logging configuration. May be you should add this to README.

@FATruden FATruden closed this Oct 3, 2019
@tcalmant
Copy link
Owner

tcalmant commented Oct 3, 2019

You're right, I'll add it to the README file

@FATruden
Copy link
Author

FATruden commented Oct 3, 2019

Thank you!

tcalmant added a commit that referenced this pull request Oct 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants