Skip to content
This repository was archived by the owner on May 13, 2022. It is now read-only.

Conversation

@AlexCato
Copy link
Contributor

@AlexCato AlexCato commented Oct 9, 2016

Currently, this is only intended for users to better understand which log messages are relevant to them.
Change all log.debug() lines in every .py file to the appropriate log level (log.debug(), log.info(), log.warn() or log.error()).
No changes in any program logic. DEBUG is also still default log level for both the console and logfiles.

AlexCato added 4 commits July 27, 2016 22:08
level (log.info(), log.warn() or log.error()).
No changes in any program logic. DEBUG also still default log level.
…nt to them.

Change all log.debug() lines in every .py file to the appropriate log level (log.debug(), log.info(), log.warn() or log.error()).
No changes in any program logic. DEBUG also still default log level.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 82.398% when pulling 03fbcfd on AlexCato:develop into 55ff9f1 on JoinMarket-Org:develop.

@AlexCato
Copy link
Contributor Author

AlexCato commented Oct 9, 2016

As a next step, I think there should be an option in joinmarket.cfg to set the desired log level for console outputs (file logs should always use DEBUG, imho). It could default to the INFO level, unless explicitly changed.
This should be especially helpful for new users, who then do not get flooded with messages which are not relevant to them.

@AdamISZ
Copy link
Member

AdamISZ commented Oct 9, 2016

It's great to see someone kicked this off finally! :)

Now let's start bikeshedding :)

I had the same thought about the logfile. I'll try to make some comments tomorrow.

return
else:
log.debug("Failed to send message to: " + str(nick) + \
log.warn("Failed to send message to: " + str(nick) + \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one could happen very often e.g. if there is a spy connecting/disconnecting. I think info or debug would be fine, but I don't mind.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't mind either way, assumed this doesnt happen often since I have not seen that message yet. I'd favor info.

orders_fees = sorted(orders_fees, key=feekey) #sort by ascending cjfee

log.debug('considered orders = \n' + '\n'.join([str(o) for o in orders_fees
log.info('considered orders = \n' + '\n'.join([str(o) for o in orders_fees
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think keep the considered orders as debug? Seems like the class of stuff that is filling up the terminal a bit too much.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

@AdamISZ
Copy link
Member

AdamISZ commented Oct 11, 2016

Shouldn't we add to this PR the code that filters out the logging that comes out to the terminal?

@AlexCato
Copy link
Contributor Author

I've already worked on the code to filter the console messages, but from what i've researched it cant be done in the same part of the code, if it is supposed to be a config-file-option (guess you know this, but for reference: https://docs.python.org/2/howto/logging.html#logging-advanced-tutorial ). The PR I have in mind will make this more clear.
But I felt that the modified log messages can stand on their own, still improving user experience even if not filtered out by default. At least it lets them judge the severity of messages. Can add another commit to the PR with the log message filtering, if you guys want to see it here.

@AlexCato
Copy link
Contributor Author

Added the code now as well, to tweak logging to the console to INFO by default. Added this as a config option to joinmarket.cfg.
As usual there's some fights of git branches vs. me, so there's one more commit than necessary. The code for the logging changes is only in the files support.py and configure.py

Reasoning for moving the console logging initialization from support.py to configure.py:
Before my changes, the console logging was initialized before the configuration was loaded. Accoding to my 'research', it's not possible to change the log level of an already attached loghandler. So it needs to be initialized correctly the first time. That's why I moved it.

I've tested this code with YG-PE and sendpayment. Worked just fine.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 82.394% when pulling 481d9a4 on AlexCato:develop into 55ff9f1 on JoinMarket-Org:develop.

@AdamISZ AdamISZ merged commit 481d9a4 into JoinMarket-Org:develop Oct 15, 2016
AdamISZ added a commit that referenced this pull request Oct 15, 2016
2f95d88 Implement console log level filterting. Reads option from joinmarket.cfg    and default to INFO for console output. Logfiles are still DEBUG. (Alex Cato)
033662f Modify two levels as discussed in #631 (Alex Cato)
4ff5519 Change all log.debug() lines in every .py file to the appropriate log level (log.info(), log.warn() or log.error()). No changes in any program logic. DEBUG also still default log level. (Alex Cato)
03fbcfd Intended for users to better understand which log messages are relevant to them. Change all log.debug() lines in every .py file to the appropriate log level (log.debug(), log.info(), log.warn() or log.error()). No changes in any program logic. DEBUG also still default log level. (Alex Cato)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants