-
Notifications
You must be signed in to change notification settings - Fork 7
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
Various bugfixes #2
base: master
Are you sure you want to change the base?
Conversation
bots/communication.py
Outdated
@@ -153,7 +153,7 @@ def run(self): | |||
# ~ #max_nr_retry : get this from channel. should be integer, but only textfields where left. so might be ''/None->use 0 | |||
# ~ max_nr_retry = int(self.channeldict['rsrv1']) if self.channeldict['rsrv1'] else 0 | |||
# ~ if max_nr_retry: | |||
# ~ domain = (self.channeldict['idchannel'] + u'_failure')[:35] | |||
# ~ domain = (self.channeldict['idchannel'] + '_failure')[:35] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
im not sure about why this section is commented out but updated the code anyway in case it goes back in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The else block should go before finally, then un-comment it.
Logic here is to only report when max_nr_retry errors in a row. As soon as there is a successful connection the counter should be reset to zero
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also like the idea of max_nr_connect_tries; this will help with transient timeouts but not if a server is really down for some period.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed this, see commit f2eec06
syntax error indent
fix ini key for daily logging to match botsinit.py print to console all severe errors
@@ -196,6 +198,7 @@ def start(): | |||
|
|||
cleanup.cleanup(do_cleanup_parameter,userscript,scriptname) | |||
except Exception as msg: | |||
print('Severe error:' + msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed, is via logger
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added these for quicker debugging while testing, it's good to see errors on the console
for key,value in botslib.botsinfo(): #log info about environement, versions, etc | ||
botsglobal.logger.info('%(key)s: "%(value)s".',{'key':key,'value':value}) | ||
|
||
#**************connect to database********************************** | ||
try: | ||
botsinit.connect() | ||
except Exception as msg: | ||
print('Error connecting to database:' + msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed, is via logger
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added these for quicker debugging while testing, it's good to see errors on the console
for key,value in botslib.botsinfo(): #log info about environement, versions, etc | ||
botsglobal.logger.info('%(key)s: "%(value)s".',{'key':key,'value':value}) | ||
|
||
#**************connect to database********************************** | ||
try: | ||
botsinit.connect() | ||
except Exception as msg: | ||
print('Error connecting to database:' + msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed, is via logger
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added these for quicker debugging while testing, it's good to see errors on the console
botslib.sendbotserrorreport('[Bots severe error] Bots is not running anymore','Bots does not run because logging is not possible.\nOften a rights problem.\n') | ||
sys.exit(1) | ||
else: | ||
if botsglobal.ini.get('settings','log_file_number','') != 'daily': | ||
if botsglobal.ini.get('settings','log_when',None) != 'daily': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure I understand this. there is no 'log_when'. if new, what does it do?
it would be good to have an option to log engine to one log file (per day), as you have suggested..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought you had added log_when? It is there in botsinit so I changed engine to use it also.
Probably changed because botsglobal.ini.getint('settings','log_file_number',10) gives an error because 'daily' is not an int.
Also means you can set daily AND number of days to keep.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally made this change years ago and probably posted to the mailing list, but only used log_file_number at that time. In my installations I keep daily logs and do daily reporting on those in botsengine postcleanup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs to be added in bots.ini but comment out by default
#log_file_number: number of rotating log files. Value: number. Default: 10
log_file_number = 10
#for one merged log per day, set log_when = daily. Default: Each run uses it's own log file
#log_when = daily
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could also use a setting in bots.ini to explicitly state the default; it will not affect operation of this setting.
#for one merged log per day, set log_when = daily. Default: Each run uses it's own log file
log_when = each run
@@ -85,17 +85,19 @@ def start(): | |||
botsglobal.logger = botsinit.initenginelogging(process_name) | |||
atexit.register(logging.shutdown) | |||
except Exception as msg: | |||
print('Error initialising logging:' + msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed, is via logger
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but this error is during log init, so never gets logged!
(I had this error a long time ago and took me a while to track down what caused it... but can't remember what it was now. Maybe permissions on the log folder)
1. match variable names to the settings - less confusing reading the code :-) 2. maxconnectiontries applies to both in and out channels. 3. fix the try-except-else-finally (i had to read python docs again... break in finally clause was no good) 4. new python 3 exception context adds confusion to the trace when there is a connection error; fix that to just re-raise the original (raise exc from exc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
91d209a commit doesn't properly fix it, the next one does though
No description provided.