On 08/31/2016 06:06 PM, Lucio Correia wrote:

Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/control/base.py | 47 +++++++++++++++++------------------------------ src/wok/root.py | 41 ++++------------------------------------- 2 files changed, 21 insertions(+), 67 deletions(-)

diff --git a/src/wok/control/base.py b/src/wok/control/base.py index 04cf2cb..f4443c8 100644 --- a/src/wok/control/base.py +++ b/src/wok/control/base.py @@ -43,6 +43,20 @@ RESOURCE_DEFAULT_LOG = "WOKRES0001L" LOG_DISABLED_METHODS = ['GET']

+def log_request(code, params, exception, method, status, user=None): + log_id = RequestRecord( + {'code': code, 'params': params}, + exception, + app=get_plugin_from_request(), + req=method, + status=status, + user=user or cherrypy.session.get(USER_NAME, 'N/A'), + ip=cherrypy.request.remote.ip + ).log() + + return log_id + +

As this function may be used by everyone, I suggest to move it to control/utils.py

class Resource(object): """ A Resource represents a single entity in the API (such as a Virtual @@ -157,16 +171,7 @@ class Resource(object): # log request code = self.getRequestMessage(method, action_name) reqParams = utf8_dict(self.log_args, request) - RequestRecord( - reqParams, - details, - app=get_plugin_from_request(), - msgCode=code, - req=method, - status=status, - user=cherrypy.session.get(USER_NAME, 'N/A'), - ip=cherrypy.request.remote.ip - ).log() + log_request(code, reqParams, details, method, status)

wrapper.__name__ = action_name wrapper.exposed = True @@ -220,16 +225,7 @@ class Resource(object): # log request if method not in LOG_DISABLED_METHODS: code = self.getRequestMessage(method) - RequestRecord( - self.log_args, - details, - app=get_plugin_from_request(), - msgCode=code, - req=method, - status=status, - user=cherrypy.session.get(USER_NAME, 'N/A'), - ip=cherrypy.request.remote.ip - ).log() + log_request(code, self.log_args, details, method, status)

return result

@@ -462,16 +458,7 @@ class Collection(object): # log request code = self.getRequestMessage(method) reqParams = utf8_dict(self.log_args, params) - RequestRecord( - reqParams, - details, - app=get_plugin_from_request(), - msgCode=code, - req=method, - status=status, - user=cherrypy.session.get(USER_NAME, 'N/A'), - ip=cherrypy.request.remote.ip - ).log() + log_request(code, reqParams, details, method, status)

class AsyncCollection(Collection): diff --git a/src/wok/root.py b/src/wok/root.py index e54bf38..8f2f764 100644 --- a/src/wok/root.py +++ b/src/wok/root.py @@ -29,10 +29,9 @@ from wok import template from wok.i18n import messages from wok.config import paths as wok_paths from wok.control import sub_nodes -from wok.control.base import Resource +from wok.control.base import log_request, Resource from wok.control.utils import parse_request from wok.exception import MissingParameter -from wok.reqlogger import RequestRecord

ROOT_REQUESTS = { @@ -156,8 +155,6 @@ class WokRoot(Root): details = None method = 'POST' code = self.getRequestMessage(method, 'login') - app = 'wok' - ip = cherrypy.request.remote.ip

try: params = parse_request() @@ -165,18 +162,7 @@ class WokRoot(Root): password = params['password'] except KeyError, item: details = e = MissingParameter('WOKAUTH0003E', {'item': str(item)}) - - RequestRecord( - params, - details, - app=app, - msgCode=code, - req=method, - status=400, - user='N/A', - ip=ip - ).log() - + log_request(code, params, details, method, 400) raise cherrypy.HTTPError(400, e.message)

try: @@ -186,16 +172,7 @@ class WokRoot(Root): status = e.status raise finally: - RequestRecord( - params, - details, - app=app, - msgCode=code, - req=method, - status=status, - user='N/A', - ip=ip - ).log() + log_request(code, params, details, method, status)

return json.dumps(user_info)

@@ -204,19 +181,9 @@ class WokRoot(Root): method = 'POST' code = self.getRequestMessage(method, 'logout') params = {'username': cherrypy.session.get(auth.USER_NAME, 'N/A')} - ip = cherrypy.request.remote.ip

auth.logout()

- RequestRecord( - params, - None, - app='wok', - msgCode=code, - req=method, - status=200, - user=params['username'], - ip=ip - ).log() + log_request(code, params, None, method, 200, user=params['username'])

return '{}'

On 08/31/2016 06:06 PM, Lucio Correia wrote:

Currently get_plugin_from_request() assumes plugin's root URI is always /plugins/<plugin_name>, which is in fact configurable by each plugin.

This patch fixes it by mapping plugins' root URI on load time, and replacing get_plugin_from_request() by a more generic get_plugin_from_uri() to be used also in AsyncTask.

Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/control/base.py | 4 ++-- src/wok/utils.py | 14 +++++++++----- 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/wok/control/base.py b/src/wok/control/base.py index f4443c8..213e6b5 100644 --- a/src/wok/control/base.py +++ b/src/wok/control/base.py @@ -33,7 +33,7 @@ from wok.control.utils import validate_params from wok.exception import InvalidOperation, UnauthorizedError, WokException from wok.reqlogger import RequestRecord from wok.stringutils import encode_value, utf8_dict -from wok.utils import get_plugin_from_request, wok_log +from wok.utils import get_plugin_from_uri, wok_log

# Default request log messages @@ -47,7 +47,7 @@ def log_request(code, params, exception, method, status, user=None): log_id = RequestRecord( {'code': code, 'params': params}, exception, - app=get_plugin_from_request(),

+ app=get_plugin_from_uri(cherrypy.request.script_name),

Maybe you can use "cherrypy.request.app.root.domain" directly or... (see below)

req=method, status=status, user=user or cherrypy.session.get(USER_NAME, 'N/A'), diff --git a/src/wok/utils.py b/src/wok/utils.py index 7599e85..b3d252c 100644 --- a/src/wok/utils.py +++ b/src/wok/utils.py @@ -44,6 +44,7 @@ from wok.exception import InvalidParameter, TimeoutExpired from wok.stringutils import decode_value

+plugin_root = {} wok_log = cherrypy.log.error_log

@@ -82,6 +83,10 @@ def get_enabled_plugins(): plugin_config = _load_plugin_conf(name) try: if plugin_config['wok']['enable']:

+ # update plugin lookup dict + uri = plugin_config['wok']['uri'] + plugin_root[name] = uri +

It is better to have the uri as the key. So you can easily grab the application name based on uri. plugin_root[uri] = name

yield (name, plugin_config) except (TypeError, KeyError): continue @@ -106,14 +111,13 @@ def get_all_tabs(): return tabs

-def get_plugin_from_request(): +def get_plugin_from_uri(uri): """ Returns name of plugin being requested. If no plugin, returns 'wok'. """

- script_name = cherrypy.request.script_name - split = script_name.split('/') - if len(split) > 2 and split[1] == 'plugins': - return split[2] + for plugin, root in plugin_root.iteritems(): + if uri.startswith(root): + return plugin

return 'wok'

Replase all the above block to: return plugin_root.get(uri, 'wok')

On 01-09-2016 10:25, Aline Manera wrote:

On 08/31/2016 06:06 PM, Lucio Correia wrote:

...

Currently get_plugin_from_request() assumes plugin's root URI is always /plugins/<plugin_name>, which is in fact configurable by each plugin.

This patch fixes it by mapping plugins' root URI on load time, and replacing get_plugin_from_request() by a more generic get_plugin_from_uri() to be used also in AsyncTask.

Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/control/base.py | 4 ++-- src/wok/utils.py | 14 +++++++++----- 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/wok/control/base.py b/src/wok/control/base.py index f4443c8..213e6b5 100644 --- a/src/wok/control/base.py +++ b/src/wok/control/base.py @@ -33,7 +33,7 @@ from wok.control.utils import validate_params from wok.exception import InvalidOperation, UnauthorizedError, WokException from wok.reqlogger import RequestRecord from wok.stringutils import encode_value, utf8_dict -from wok.utils import get_plugin_from_request, wok_log +from wok.utils import get_plugin_from_uri, wok_log

# Default request log messages @@ -47,7 +47,7 @@ def log_request(code, params, exception, method, status, user=None): log_id = RequestRecord( {'code': code, 'params': params}, exception, - app=get_plugin_from_request(),

...

+ app=get_plugin_from_uri(cherrypy.request.script_name),

Maybe you can use "cherrypy.request.app.root.domain" directly or... (see below)

...

req=method, status=status, user=user or cherrypy.session.get(USER_NAME, 'N/A'), diff --git a/src/wok/utils.py b/src/wok/utils.py index 7599e85..b3d252c 100644 --- a/src/wok/utils.py +++ b/src/wok/utils.py @@ -44,6 +44,7 @@ from wok.exception import InvalidParameter, TimeoutExpired from wok.stringutils import decode_value

+plugin_root = {} wok_log = cherrypy.log.error_log

@@ -82,6 +83,10 @@ def get_enabled_plugins(): plugin_config = _load_plugin_conf(name) try: if plugin_config['wok']['enable']:

...

+ # update plugin lookup dict + uri = plugin_config['wok']['uri'] + plugin_root[name] = uri +

It is better to have the uri as the key. So you can easily grab the application name based on uri.

plugin_root[uri] = name

That was the original idea, but I did this way because the usage of asynctask (it needed starts with to deal with target_uri). Since you gave me an alternative for that usage, I will test it and can change this afterwards.

...

yield (name, plugin_config) except (TypeError, KeyError): continue @@ -106,14 +111,13 @@ def get_all_tabs(): return tabs

-def get_plugin_from_request(): +def get_plugin_from_uri(uri): """ Returns name of plugin being requested. If no plugin, returns 'wok'. """

...

- script_name = cherrypy.request.script_name - split = script_name.split('/') - if len(split) > 2 and split[1] == 'plugins': - return split[2] + for plugin, root in plugin_root.iteritems(): + if uri.startswith(root): + return plugin

return 'wok'

Replase all the above block to:

return plugin_root.get(uri, 'wok')

OK if the above works. -- Lucio Correia Software Engineer IBM LTC Brazil

On 01-09-2016 10:35, Aline Manera wrote:

On 08/31/2016 06:06 PM, Lucio Correia wrote:

...

* Add ID to each entry * Change log entry format to one JSON per line * Modify getRecords() to apply AsyncTask entry status to original request entry

Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/message.py | 2 +- src/wok/reqlogger.py | 142 +++++++++++++++++++++++++++++---------------------- 2 files changed, 82 insertions(+), 62 deletions(-)

diff --git a/src/wok/message.py b/src/wok/message.py index ff4cbc9..28462b7 100644 --- a/src/wok/message.py +++ b/src/wok/message.py @@ -51,7 +51,7 @@ class WokMessage(object): wok_app = cherrypy.tree.apps.get('', None)

# get app from plugin path if specified

...

- if self.plugin: + if self.plugin and self.plugin != 'wok':

Why is that needed? Why wok is different from the rest of the application?

This is consequence of returning 'wok' as default instead of None in get_plugin_from_request. I will rethink it in case I can return None there.

...

app = cherrypy.tree.apps.get(self.plugin, None) # if on request, try to get app from it elif cherrypy.request.app: diff --git a/src/wok/reqlogger.py b/src/wok/reqlogger.py index 9f1d2c8..2923d7c 100644 --- a/src/wok/reqlogger.py +++ b/src/wok/reqlogger.py @@ -24,6 +24,7 @@ import logging import logging.handlers import os.path import time +import uuid

from cherrypy.process.plugins import BackgroundTask from tempfile import NamedTemporaryFile @@ -32,7 +33,7 @@ from wok.config import config, get_log_download_path from wok.exception import InvalidParameter, OperationFailed from wok.message import WokMessage from wok.stringutils import ascii_dict -from wok.utils import remove_old_files +from wok.utils import plugin_root, remove_old_files

# Log search setup @@ -62,6 +63,9 @@ UNSAFE_REQUEST_PARAMETERS = ['password', 'passwd'] REQUEST_LOG_FILE = "wok-req.log" WOK_REQUEST_LOGGER = "wok_request_logger"

...

+# AsyncTask handling +ASYNCTASK_REQUEST_METHOD = 'TASK' +

I am curious to see how this constant is used.

...

class RequestLogger(object): def __init__(self): @@ -114,19 +118,17 @@ class RequestParser(object):

return LOG_DOWNLOAD_URI % os.path.basename(fd.name)

- def getTranslatedMessage(self, record, params, detParams): - code = record.get('msgCode', '') - app = record.get('app', 'wok') - plugin = None - if app != 'wok': - plugin = "/plugins/%s" % app -

...

+ def getTranslatedMessage(self, message, error, app): + plugin = plugin_root.get(app, 'wok')

Do not set 'wok' as plugin name. It is not true as wok is not a plugin.

The var is named plugin but it means app.. wok or any plugin. I may change the variable name.

...

+ code = message.get('code', '') + params = message.get('params', {}) msg = WokMessage(code, params, plugin) text = msg.get_text(prepend_code=False, translate=True)

- detCode = record.get('detCode', '') - if detCode: - msg = WokMessage(detCode, detParams, plugin) + if error: + code = error.get('code', '') + params = error.get('params', {}) + msg = WokMessage(code, params, plugin) text += ' ' + msg.get_text(prepend_code=True, translate=True)

return text @@ -137,8 +139,40 @@ class RequestParser(object): for filename in glob.glob(self.baseFile + "-*[!.gz]"): records.extend(self.getRecordsFromFile(filename))

- # Return ordered by latest events first - return sorted(records, key=lambda k: k['date'] + k['time'], + # sort records chronologically + sort = sorted(records, key=lambda k: k['info']['date'] + + k['info']['time']) +

...

+ # normalize entries + normalized = {} + asynctask_updates = [] + for record in sort:

...

+ # async task entries are left to the end

Why?

Because they are just updates to the original request log entry. And the task log entry may be written before the task finishes since runs in separate thread.

...

+ if record['info']['req'] == ASYNCTASK_REQUEST_METHOD: + asynctask_updates.append(record) + continue +

Could you explain why do we need to add an specific 'req' information for task?

I use it to differentiate task finish log entries from regular request log entries.

...

+ # initial request entry: generate message text + message = record.pop('message') + error = record.pop('error', None) + plugin = record['info']['app'] + text = self.getTranslatedMessage(message, error, plugin) + record['info']['message'] = text + + id = record.pop('id') + normalized[id] = record['info'] + + # async task entries: update only status + for record in asynctask_updates: + id = record.pop('id') + + # task id may not exist, since GET requests are not logged but + # may generate tasks (i.e. AsyncResource) + if id in normalized: + normalized[id]['status'] = record['info']['status'] + + # return results in chronological reverse order + return sorted(normalized.values(), key=lambda k: k['date'] + k['time'], reverse=True)

def getRecordsFromFile(self, filename): @@ -151,34 +185,12 @@ class RequestParser(object): if not os.path.exists(filename): return []

- # read records from file try: with open(filename) as f: line = f.readline() while line != "": - data = line.split(">>>") - if len(data) > 1: - record = json.JSONDecoder().decode(data[0]) - - if len(data) > 2: - # new log format: translate message on the fly - params = json.JSONDecoder().decode(data[1]) - detParams = None - - # has original exception details - if len(data) > 3: - detParams = json.JSONDecoder().decode(data[2]) - - msg = self.getTranslatedMessage(record, params, - detParams) - else: - # make it compatible with v2.2 log files, which - # messages are already translated - msg = data[1].strip() - - record['message'] = msg - records.append(record) - + record = json.JSONDecoder().decode(line) + records.append(record) line = f.readline()

f. close() @@ -217,20 +229,32 @@ class RequestParser(object):

class RequestRecord(object): - def __init__(self, msgParams, details, **kwargs): - # log record data - self.logData = kwargs - self.details = details - + def __init__(self, message, error, id=None, **kwargs): # data for message translation - self.code = self.logData['msgCode'] - self.params = self.getSafeReqParams(msgParams) - - # register timestamp in local time + self.message = { + 'code': message['code'], + 'params': self.getSafeReqParams(message['params']), + } + + # data for error translation (WokException) + self.error = None + if error: + self.error = { + 'code': error.code, + 'params': error.params, + } + + # log entry info + self.id = id or str(uuid.uuid4()) + self.info = kwargs + + # generate timestamp timestamp = time.localtime() - self.logData['date'] = time.strftime(TS_DATE_FORMAT, timestamp) - self.logData['time'] = time.strftime(TS_TIME_FORMAT, timestamp) - self.logData['zone'] = time.strftime(TS_ZONE_FORMAT, timestamp) + self.info.update({ + 'date': time.strftime(TS_DATE_FORMAT, timestamp), + 'time': time.strftime(TS_TIME_FORMAT, timestamp), + 'zone': time.strftime(TS_ZONE_FORMAT, timestamp), + })

def getSafeReqParams(self, params): result = params.copy() @@ -239,20 +263,16 @@ class RequestRecord(object): return result

def __str__(self): - # original exception details - details = '' - if self.details: - self.logData['detCode'] = self.details.code - details = ">>> %s" % json.JSONEncoder().encode(self.details.params) - - # request log message - msg = WokMessage(self.code, self.params) - msgText = msg.get_text(prepend_code=False, translate=False) - logData = json.JSONEncoder().encode(self.logData) - msgParams = json.JSONEncoder().encode(self.params) + entry = { + "id": self.id, + "message": self.message, + "error": self.error, + "info": self.info, + }

- return '%s >>> %s %s >>> %s' % (logData, msgParams, details, msgText) + return json.JSONEncoder().encode(entry)

def log(self): reqLogger = logging.getLogger(WOK_REQUEST_LOGGER) reqLogger.info(self) + return self.id

-- Lucio Correia Software Engineer IBM LTC Brazil

On 01-09-2016 13:19, Aline Manera wrote:

On 09/01/2016 12:56 PM, Lucio Correia wrote:

...

On 01-09-2016 10:35, Aline Manera wrote:

...

On 08/31/2016 06:06 PM, Lucio Correia wrote:

...

* Add ID to each entry * Change log entry format to one JSON per line * Modify getRecords() to apply AsyncTask entry status to original request entry

Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/message.py | 2 +- src/wok/reqlogger.py | 142 +++++++++++++++++++++++++++++---------------------- 2 files changed, 82 insertions(+), 62 deletions(-)

diff --git a/src/wok/message.py b/src/wok/message.py index ff4cbc9..28462b7 100644 --- a/src/wok/message.py +++ b/src/wok/message.py @@ -51,7 +51,7 @@ class WokMessage(object): wok_app = cherrypy.tree.apps.get('', None)

# get app from plugin path if specified

...

- if self.plugin: + if self.plugin and self.plugin != 'wok':

Why is that needed? Why wok is different from the rest of the application?

This is consequence of returning 'wok' as default instead of None in get_plugin_from_request. I will rethink it in case I can return None there.

...

...

app = cherrypy.tree.apps.get(self.plugin, None) # if on request, try to get app from it elif cherrypy.request.app: diff --git a/src/wok/reqlogger.py b/src/wok/reqlogger.py index 9f1d2c8..2923d7c 100644 --- a/src/wok/reqlogger.py +++ b/src/wok/reqlogger.py @@ -24,6 +24,7 @@ import logging import logging.handlers import os.path import time +import uuid

from cherrypy.process.plugins import BackgroundTask from tempfile import NamedTemporaryFile @@ -32,7 +33,7 @@ from wok.config import config, get_log_download_path from wok.exception import InvalidParameter, OperationFailed from wok.message import WokMessage from wok.stringutils import ascii_dict -from wok.utils import remove_old_files +from wok.utils import plugin_root, remove_old_files

# Log search setup @@ -62,6 +63,9 @@ UNSAFE_REQUEST_PARAMETERS = ['password', 'passwd'] REQUEST_LOG_FILE = "wok-req.log" WOK_REQUEST_LOGGER = "wok_request_logger"

...

+# AsyncTask handling +ASYNCTASK_REQUEST_METHOD = 'TASK' +

I am curious to see how this constant is used.

...

class RequestLogger(object): def __init__(self): @@ -114,19 +118,17 @@ class RequestParser(object):

return LOG_DOWNLOAD_URI % os.path.basename(fd.name)

- def getTranslatedMessage(self, record, params, detParams): - code = record.get('msgCode', '') - app = record.get('app', 'wok') - plugin = None - if app != 'wok': - plugin = "/plugins/%s" % app -

...

+ def getTranslatedMessage(self, message, error, app): + plugin = plugin_root.get(app, 'wok')

Do not set 'wok' as plugin name. It is not true as wok is not a plugin.

The var is named plugin but it means app.. wok or any plugin. I may change the variable name.

...

...

+ code = message.get('code', '') + params = message.get('params', {}) msg = WokMessage(code, params, plugin) text = msg.get_text(prepend_code=False, translate=True)

- detCode = record.get('detCode', '') - if detCode: - msg = WokMessage(detCode, detParams, plugin) + if error: + code = error.get('code', '') + params = error.get('params', {}) + msg = WokMessage(code, params, plugin) text += ' ' + msg.get_text(prepend_code=True, translate=True)

return text @@ -137,8 +139,40 @@ class RequestParser(object): for filename in glob.glob(self.baseFile + "-*[!.gz]"): records.extend(self.getRecordsFromFile(filename))

- # Return ordered by latest events first - return sorted(records, key=lambda k: k['date'] + k['time'], + # sort records chronologically + sort = sorted(records, key=lambda k: k['info']['date'] + + k['info']['time']) +

...

+ # normalize entries + normalized = {} + asynctask_updates = [] + for record in sort:

...

+ # async task entries are left to the end

Why?

Because they are just updates to the original request log entry. And the task log entry may be written before the task finishes since runs in separate thread.

Ok. Please, add more comments related to that so it is easy to understand the code proposal.

OK

...

...

...

+ if record['info']['req'] == ASYNCTASK_REQUEST_METHOD: + asynctask_updates.append(record) + continue +

Could you explain why do we need to add an specific 'req' information for task?

I use it to differentiate task finish log entries from regular request log entries.

You can use the status parameter for that. We don't need an extra parameter.

I'm not creating a new parameter, I'm using method (request type) instead of status. So instead of POST, PUT, it will be TASK. It's the same right? Or there is a reason to not use request type? This is not showed to the user.

...

...

...

+ # initial request entry: generate message text + message = record.pop('message') + error = record.pop('error', None) + plugin = record['info']['app'] + text = self.getTranslatedMessage(message, error, plugin) + record['info']['message'] = text + + id = record.pop('id') + normalized[id] = record['info'] + + # async task entries: update only status + for record in asynctask_updates: + id = record.pop('id') + + # task id may not exist, since GET requests are not logged but + # may generate tasks (i.e. AsyncResource) + if id in normalized: + normalized[id]['status'] = record['info']['status'] + + # return results in chronological reverse order + return sorted(normalized.values(), key=lambda k: k['date'] + k['time'], reverse=True)

def getRecordsFromFile(self, filename): @@ -151,34 +185,12 @@ class RequestParser(object): if not os.path.exists(filename): return []

- # read records from file try: with open(filename) as f: line = f.readline() while line != "": - data = line.split(">>>") - if len(data) > 1: - record = json.JSONDecoder().decode(data[0]) - - if len(data) > 2: - # new log format: translate message on the fly - params = json.JSONDecoder().decode(data[1]) - detParams = None - - # has original exception details - if len(data) > 3: - detParams = json.JSONDecoder().decode(data[2]) - - msg = self.getTranslatedMessage(record, params, - detParams) - else: - # make it compatible with v2.2 log files, which - # messages are already translated - msg = data[1].strip() - - record['message'] = msg - records.append(record) - + record = json.JSONDecoder().decode(line) + records.append(record) line = f.readline()

f. close() @@ -217,20 +229,32 @@ class RequestParser(object):

class RequestRecord(object): - def __init__(self, msgParams, details, **kwargs): - # log record data - self.logData = kwargs - self.details = details - + def __init__(self, message, error, id=None, **kwargs): # data for message translation - self.code = self.logData['msgCode'] - self.params = self.getSafeReqParams(msgParams) - - # register timestamp in local time + self.message = { + 'code': message['code'], + 'params': self.getSafeReqParams(message['params']), + } + + # data for error translation (WokException) + self.error = None + if error: + self.error = { + 'code': error.code, + 'params': error.params, + } + + # log entry info + self.id = id or str(uuid.uuid4()) + self.info = kwargs + + # generate timestamp timestamp = time.localtime() - self.logData['date'] = time.strftime(TS_DATE_FORMAT, timestamp) - self.logData['time'] = time.strftime(TS_TIME_FORMAT, timestamp) - self.logData['zone'] = time.strftime(TS_ZONE_FORMAT, timestamp) + self.info.update({ + 'date': time.strftime(TS_DATE_FORMAT, timestamp), + 'time': time.strftime(TS_TIME_FORMAT, timestamp), + 'zone': time.strftime(TS_ZONE_FORMAT, timestamp), + })

def getSafeReqParams(self, params): result = params.copy() @@ -239,20 +263,16 @@ class RequestRecord(object): return result

def __str__(self): - # original exception details - details = '' - if self.details: - self.logData['detCode'] = self.details.code - details = ">>> %s" % json.JSONEncoder().encode(self.details.params) - - # request log message - msg = WokMessage(self.code, self.params) - msgText = msg.get_text(prepend_code=False, translate=False) - logData = json.JSONEncoder().encode(self.logData) - msgParams = json.JSONEncoder().encode(self.params) + entry = { + "id": self.id, + "message": self.message, + "error": self.error, + "info": self.info, + }

- return '%s >>> %s %s >>> %s' % (logData, msgParams, details, msgText) + return json.JSONEncoder().encode(entry)

def log(self): reqLogger = logging.getLogger(WOK_REQUEST_LOGGER) reqLogger.info(self) + return self.id

-- Lucio Correia Software Engineer IBM LTC Brazil

On 01-09-2016 13:48, Aline Manera wrote:

On 09/01/2016 01:31 PM, Lucio Correia wrote:

...

On 01-09-2016 13:19, Aline Manera wrote:

...

On 09/01/2016 12:56 PM, Lucio Correia wrote:

...

On 01-09-2016 10:35, Aline Manera wrote:

...

On 08/31/2016 06:06 PM, Lucio Correia wrote:

...

* Add ID to each entry * Change log entry format to one JSON per line * Modify getRecords() to apply AsyncTask entry status to original request entry

Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/message.py | 2 +- src/wok/reqlogger.py | 142 +++++++++++++++++++++++++++++---------------------- 2 files changed, 82 insertions(+), 62 deletions(-)

diff --git a/src/wok/message.py b/src/wok/message.py index ff4cbc9..28462b7 100644 --- a/src/wok/message.py +++ b/src/wok/message.py @@ -51,7 +51,7 @@ class WokMessage(object): wok_app = cherrypy.tree.apps.get('', None)

# get app from plugin path if specified

...

- if self.plugin: + if self.plugin and self.plugin != 'wok':

Why is that needed? Why wok is different from the rest of the application?

This is consequence of returning 'wok' as default instead of None in get_plugin_from_request. I will rethink it in case I can return None there.

...

...

app = cherrypy.tree.apps.get(self.plugin, None) # if on request, try to get app from it elif cherrypy.request.app: diff --git a/src/wok/reqlogger.py b/src/wok/reqlogger.py index 9f1d2c8..2923d7c 100644 --- a/src/wok/reqlogger.py +++ b/src/wok/reqlogger.py @@ -24,6 +24,7 @@ import logging import logging.handlers import os.path import time +import uuid

from cherrypy.process.plugins import BackgroundTask from tempfile import NamedTemporaryFile @@ -32,7 +33,7 @@ from wok.config import config, get_log_download_path from wok.exception import InvalidParameter, OperationFailed from wok.message import WokMessage from wok.stringutils import ascii_dict -from wok.utils import remove_old_files +from wok.utils import plugin_root, remove_old_files

# Log search setup @@ -62,6 +63,9 @@ UNSAFE_REQUEST_PARAMETERS = ['password', 'passwd'] REQUEST_LOG_FILE = "wok-req.log" WOK_REQUEST_LOGGER = "wok_request_logger"

...

+# AsyncTask handling +ASYNCTASK_REQUEST_METHOD = 'TASK' +

I am curious to see how this constant is used.

...

class RequestLogger(object): def __init__(self): @@ -114,19 +118,17 @@ class RequestParser(object):

return LOG_DOWNLOAD_URI % os.path.basename(fd.name)

- def getTranslatedMessage(self, record, params, detParams): - code = record.get('msgCode', '') - app = record.get('app', 'wok') - plugin = None - if app != 'wok': - plugin = "/plugins/%s" % app -

...

+ def getTranslatedMessage(self, message, error, app): + plugin = plugin_root.get(app, 'wok')

Do not set 'wok' as plugin name. It is not true as wok is not a plugin.

The var is named plugin but it means app.. wok or any plugin. I may change the variable name.

...

...

+ code = message.get('code', '') + params = message.get('params', {}) msg = WokMessage(code, params, plugin) text = msg.get_text(prepend_code=False, translate=True)

- detCode = record.get('detCode', '') - if detCode: - msg = WokMessage(detCode, detParams, plugin) + if error: + code = error.get('code', '') + params = error.get('params', {}) + msg = WokMessage(code, params, plugin) text += ' ' + msg.get_text(prepend_code=True, translate=True)

return text @@ -137,8 +139,40 @@ class RequestParser(object): for filename in glob.glob(self.baseFile + "-*[!.gz]"): records.extend(self.getRecordsFromFile(filename))

- # Return ordered by latest events first - return sorted(records, key=lambda k: k['date'] + k['time'], + # sort records chronologically + sort = sorted(records, key=lambda k: k['info']['date'] + + k['info']['time']) +

...

+ # normalize entries + normalized = {} + asynctask_updates = [] + for record in sort:

...

+ # async task entries are left to the end

Why?

Because they are just updates to the original request log entry. And the task log entry may be written before the task finishes since runs in separate thread.

Ok. Please, add more comments related to that so it is easy to understand the code proposal.

OK

...

...

...

...

+ if record['info']['req'] == ASYNCTASK_REQUEST_METHOD: + asynctask_updates.append(record) + continue +

Could you explain why do we need to add an specific 'req' information for task?

I use it to differentiate task finish log entries from regular request log entries.

You can use the status parameter for that. We don't need an extra parameter.

I'm not creating a new parameter, I'm using method (request type) instead of status. So instead of POST, PUT, it will be TASK.

It's the same right? Or there is a reason to not use request type? This is not showed to the user.

I'd say to do not mix things. A parameter related to HTTP methods should not handle any other information. Why do not use the status code? Wouldn't be the status code used by UI to update the information to user?

The TASK type log entries are not in the final log - UI does not see them. They are filtered by backend before, exactly what this code is doing here. In the task log entry, I use status=200 or status=400 (see async task _log() usage) to mean success or failure, and here I use that value to update the original request log entry (which contains 202 until task finishes).

...

...

...

...

...

+ # initial request entry: generate message text + message = record.pop('message') + error = record.pop('error', None) + plugin = record['info']['app'] + text = self.getTranslatedMessage(message, error, plugin) + record['info']['message'] = text + + id = record.pop('id') + normalized[id] = record['info'] + + # async task entries: update only status + for record in asynctask_updates: + id = record.pop('id') + + # task id may not exist, since GET requests are not logged but + # may generate tasks (i.e. AsyncResource) + if id in normalized: + normalized[id]['status'] = record['info']['status'] + + # return results in chronological reverse order + return sorted(normalized.values(), key=lambda k: k['date'] + k['time'], reverse=True)

def getRecordsFromFile(self, filename): @@ -151,34 +185,12 @@ class RequestParser(object): if not os.path.exists(filename): return []

- # read records from file try: with open(filename) as f: line = f.readline() while line != "": - data = line.split(">>>") - if len(data) > 1: - record = json.JSONDecoder().decode(data[0]) - - if len(data) > 2: - # new log format: translate message on the fly - params = json.JSONDecoder().decode(data[1]) - detParams = None - - # has original exception details - if len(data) > 3: - detParams = json.JSONDecoder().decode(data[2]) - - msg = self.getTranslatedMessage(record, params, - detParams) - else: - # make it compatible with v2.2 log files, which - # messages are already translated - msg = data[1].strip() - - record['message'] = msg - records.append(record) - + record = json.JSONDecoder().decode(line) + records.append(record) line = f.readline()

f. close() @@ -217,20 +229,32 @@ class RequestParser(object):

class RequestRecord(object): - def __init__(self, msgParams, details, **kwargs): - # log record data - self.logData = kwargs - self.details = details - + def __init__(self, message, error, id=None, **kwargs): # data for message translation - self.code = self.logData['msgCode'] - self.params = self.getSafeReqParams(msgParams) - - # register timestamp in local time + self.message = { + 'code': message['code'], + 'params': self.getSafeReqParams(message['params']), + } + + # data for error translation (WokException) + self.error = None + if error: + self.error = { + 'code': error.code, + 'params': error.params, + } + + # log entry info + self.id = id or str(uuid.uuid4()) + self.info = kwargs + + # generate timestamp timestamp = time.localtime() - self.logData['date'] = time.strftime(TS_DATE_FORMAT, timestamp) - self.logData['time'] = time.strftime(TS_TIME_FORMAT, timestamp) - self.logData['zone'] = time.strftime(TS_ZONE_FORMAT, timestamp) + self.info.update({ + 'date': time.strftime(TS_DATE_FORMAT, timestamp), + 'time': time.strftime(TS_TIME_FORMAT, timestamp), + 'zone': time.strftime(TS_ZONE_FORMAT, timestamp), + })

def getSafeReqParams(self, params): result = params.copy() @@ -239,20 +263,16 @@ class RequestRecord(object): return result

def __str__(self): - # original exception details - details = '' - if self.details: - self.logData['detCode'] = self.details.code - details = ">>> %s" % json.JSONEncoder().encode(self.details.params) - - # request log message - msg = WokMessage(self.code, self.params) - msgText = msg.get_text(prepend_code=False, translate=False) - logData = json.JSONEncoder().encode(self.logData) - msgParams = json.JSONEncoder().encode(self.params) + entry = { + "id": self.id, + "message": self.message, + "error": self.error, + "info": self.info, + }

- return '%s >>> %s %s >>> %s' % (logData, msgParams, details, msgText) + return json.JSONEncoder().encode(entry)

def log(self): reqLogger = logging.getLogger(WOK_REQUEST_LOGGER) reqLogger.info(self) + return self.id

-- Lucio Correia Software Engineer IBM LTC Brazil

On 08/31/2016 06:06 PM, Lucio Correia wrote:

* Add plugin and log_id attributes to AsyncTask * Create _log() function to log AsyncTask exit status * Create auxiliary method to save log entry ID for a task * Handle WokException separately in order to add it in translatable form to user log

Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/asynctask.py | 46 ++++++++++++++++++++++++++++++++++++++++------ src/wok/i18n.py | 2 ++ 2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/src/wok/asynctask.py b/src/wok/asynctask.py index 021b76c..633c96f 100644 --- a/src/wok/asynctask.py +++ b/src/wok/asynctask.py @@ -25,10 +25,13 @@ import time import traceback import uuid

- -from wok.exception import InvalidOperation, OperationFailed +from wok.exception import InvalidOperation, OperationFailed, WokException +from wok.reqlogger import RequestRecord, ASYNCTASK_REQUEST_METHOD +from wok.utils import get_plugin_from_uri

+MSG_FAILED = 'WOKASYNC0002L' +MSG_SUCCESS = 'WOKASYNC0001L' tasks_queue = {}

@@ -43,27 +46,54 @@ def clean_old_tasks(): task.remove()

+def save_request_log_id(log_id, task_id): + tasks_queue[task_id].log_id = log_id + +

You can use "tasks_queue[task_id].log_id = log_id " directly in the code. You are replacing one line code by one function. I don't see much gain on it specially while reviewing the code where you can grab the function to simply to understand it.

class AsyncTask(object): def __init__(self, target_uri, fn, opaque=None, kill_cb=None): + # task info self.id = str(uuid.uuid1()) self.target_uri = target_uri self.fn = fn self.kill_cb = kill_cb + self.log_id = None

+ self.plugin = get_plugin_from_uri(target_uri)

The target_uri may be anything! Usually on Kimchi, we have a pattern to be the the plugin base URI + resource URI but it may be not true to every plugin. I suggest to get that info from cherrypy

+ self.timestamp = time.time() + + # task context self.status = 'running' self.message = 'The request is being processing.' - self.timestamp = time.time() self._cp_request = cherrypy.serving.request self.thread = threading.Thread(target=self._run_helper, args=(opaque, self._status_cb)) self.thread.setDaemon(True) self.thread.start() + # let's prevent memory leak in tasks_queue clean_old_tasks() tasks_queue[self.id] = self

- def _status_cb(self, message, success=None):

+ def _log(self, code, status, exception=None): + RequestRecord( + {'code': code, 'params': {'target_uri': self.target_uri}}, + exception, + self.log_id, + app=self.plugin, + req=ASYNCTASK_REQUEST_METHOD, + status=status, + user='', + ip='', + ).log() +

Isn't this function the same you created on Patch 1? Can't you reuse the other one?

+ def _status_cb(self, message, success=None, exception=None): if success is not None: - self.status = 'finished' if success else 'failed' + if success: + self._log(MSG_SUCCESS, 200) + self.status = 'finished' + else: + self._log(MSG_FAILED, 400, exception) + self.status = 'failed'

if message.strip(): self.message = message @@ -72,10 +102,14 @@ class AsyncTask(object): cherrypy.serving.request = self._cp_request try: self.fn(cb, opaque) + except WokException, e: + cherrypy.log.error_log.error("Error in async_task %s " % self.id) + cherrypy.log.error_log.error(traceback.format_exc()) + cb(e.message, success=False, exception=e) except Exception, e: cherrypy.log.error_log.error("Error in async_task %s " % self.id) cherrypy.log.error_log.error(traceback.format_exc()) - cb(e.message, False) + cb(e.message, success=False)

def remove(self): try: diff --git a/src/wok/i18n.py b/src/wok/i18n.py index 7ba7c24..4893be7 100644 --- a/src/wok/i18n.py +++ b/src/wok/i18n.py @@ -59,6 +59,8 @@ messages = { "WOKPROXY0001E": _("Unable to (re)start system's nginx.service. Details: '%(error)s'"),

# These messages (ending with L) are for user log purposes + "WOKASYNC0001L": _("Successfully completed task '%(target_uri)s'"), + "WOKASYNC0002L": _("Failed to complete task '%(target_uri)s'"), "WOKCOL0001L": _("Request made on collection"), "WOKRES0001L": _("Request made on resource"), "WOKROOT0001L": _("User '%(username)s' login"),

On 09/01/2016 01:08 PM, Lucio Correia wrote:

On 01-09-2016 10:45, Aline Manera wrote:

...

On 08/31/2016 06:06 PM, Lucio Correia wrote:

...

* Add plugin and log_id attributes to AsyncTask * Create _log() function to log AsyncTask exit status * Create auxiliary method to save log entry ID for a task * Handle WokException separately in order to add it in translatable form to user log

Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/asynctask.py | 46 ++++++++++++++++++++++++++++++++++++++++------ src/wok/i18n.py | 2 ++ 2 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/src/wok/asynctask.py b/src/wok/asynctask.py index 021b76c..633c96f 100644 --- a/src/wok/asynctask.py +++ b/src/wok/asynctask.py @@ -25,10 +25,13 @@ import time import traceback import uuid

- -from wok.exception import InvalidOperation, OperationFailed +from wok.exception import InvalidOperation, OperationFailed, WokException +from wok.reqlogger import RequestRecord, ASYNCTASK_REQUEST_METHOD +from wok.utils import get_plugin_from_uri

+MSG_FAILED = 'WOKASYNC0002L' +MSG_SUCCESS = 'WOKASYNC0001L' tasks_queue = {}

@@ -43,27 +46,54 @@ def clean_old_tasks(): task.remove()

...

+def save_request_log_id(log_id, task_id): + tasks_queue[task_id].log_id = log_id + +

You can use "tasks_queue[task_id].log_id = log_id " directly in the code. You are replacing one line code by one function. I don't see much gain on it specially while reviewing the code where you can grab the function to simply to understand it.

Yes I could. But I did prefer to create this function due to code maintenance. This is the module that knows how to deal with tasks, so if that changes (we just did that moving tasks from objstore to memory), we change only code here and not through all the code.

Ok. I got your point.

It's also good to avoid another module to access a control structure like tasks_queue directly and do some bad stuff.

tasks_queue is a global variable =)

...

...

class AsyncTask(object): def __init__(self, target_uri, fn, opaque=None, kill_cb=None): + # task info self.id = str(uuid.uuid1()) self.target_uri = target_uri self.fn = fn self.kill_cb = kill_cb + self.log_id = None

...

+ self.plugin = get_plugin_from_uri(target_uri)

The target_uri may be anything! Usually on Kimchi, we have a pattern to be the the plugin base URI + resource URI but it may be not true to every plugin. I suggest to get that info from cherrypy

I see. I will try to use cherrypy, but I'm not sure it will work due to my past work with user logs and requests.

...

...

+ self.timestamp = time.time() + + # task context self.status = 'running' self.message = 'The request is being processing.' - self.timestamp = time.time() self._cp_request = cherrypy.serving.request self.thread = threading.Thread(target=self._run_helper, args=(opaque, self._status_cb)) self.thread.setDaemon(True) self.thread.start() + # let's prevent memory leak in tasks_queue clean_old_tasks() tasks_queue[self.id] = self

- def _status_cb(self, message, success=None):

...

+ def _log(self, code, status, exception=None): + RequestRecord( + {'code': code, 'params': {'target_uri': self.target_uri}}, + exception, + self.log_id, + app=self.plugin, + req=ASYNCTASK_REQUEST_METHOD, + status=status, + user='', + ip='', + ).log() +

Isn't this function the same you created on Patch 1? Can't you reuse the other one?

Yes, now that that function is in control/utils.py I believe I can use it here.

...

...

+ def _status_cb(self, message, success=None, exception=None): if success is not None: - self.status = 'finished' if success else 'failed' + if success: + self._log(MSG_SUCCESS, 200) + self.status = 'finished' + else: + self._log(MSG_FAILED, 400, exception) + self.status = 'failed'

if message.strip(): self.message = message @@ -72,10 +102,14 @@ class AsyncTask(object): cherrypy.serving.request = self._cp_request try: self.fn(cb, opaque) + except WokException, e: + cherrypy.log.error_log.error("Error in async_task %s " % self.id) + cherrypy.log.error_log.error(traceback.format_exc()) + cb(e.message, success=False, exception=e) except Exception, e: cherrypy.log.error_log.error("Error in async_task %s " % self.id) cherrypy.log.error_log.error(traceback.format_exc()) - cb(e.message, False) + cb(e.message, success=False)

def remove(self): try: diff --git a/src/wok/i18n.py b/src/wok/i18n.py index 7ba7c24..4893be7 100644 --- a/src/wok/i18n.py +++ b/src/wok/i18n.py @@ -59,6 +59,8 @@ messages = { "WOKPROXY0001E": _("Unable to (re)start system's nginx.service. Details: '%(error)s'"),

# These messages (ending with L) are for user log purposes + "WOKASYNC0001L": _("Successfully completed task '%(target_uri)s'"), + "WOKASYNC0002L": _("Failed to complete task '%(target_uri)s'"), "WOKCOL0001L": _("Request made on collection"), "WOKRES0001L": _("Request made on resource"), "WOKROOT0001L": _("User '%(username)s' login"),

On 01-09-2016 11:09, Aline Manera wrote:

On 08/31/2016 06:06 PM, Lucio Correia wrote:

...

Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/control/base.py | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/src/wok/control/base.py b/src/wok/control/base.py index 213e6b5..06f1f08 100644 --- a/src/wok/control/base.py +++ b/src/wok/control/base.py @@ -26,6 +26,7 @@ import urllib2

import wok.template +from wok.asynctask import save_request_log_id from wok.auth import USER_GROUPS, USER_NAME, USER_ROLES from wok.control.utils import get_class_name, internal_redirect, model_fn from wok.control.utils import parse_request, validate_method @@ -162,6 +163,7 @@ class Resource(object): self.info['persistent'] is True): result = render_fn(self, action_result) status = cherrypy.response.status + return result except WokException, e: details = e @@ -171,7 +173,9 @@ class Resource(object): # log request code = self.getRequestMessage(method, action_name) reqParams = utf8_dict(self.log_args, request) - log_request(code, reqParams, details, method, status)

...

+ log_id = log_request(code, reqParams, details, method, status) + if status == 202: + save_request_log_id(log_id, action_result['id'])

This should be only on generate_action_handler_task(), right? As this function is specific to handle Tasks.

Yes, I did it originally. But I need request information and it's not possible to call parse_request() again. So I needed to move it to here.

...

wrapper.__name__ = action_name wrapper.exposed = True @@ -223,7 +227,7 @@ class Resource(object): raise finally: # log request

...

- if method not in LOG_DISABLED_METHODS: + if method not in LOG_DISABLED_METHODS and status != 202:

Why status 202 has a different behavior?

It's logged separately in AsyncResource class methods, since it needs log entry id and task id, which is available only there.

...

code = self.getRequestMessage(method) log_request(code, self.log_args, details, method, status)

@@ -307,6 +311,15 @@ class AsyncResource(Resource): raise cherrypy.HTTPError(405, e.message)

cherrypy.response.status = 202

...

+ + # log request + method = 'DELETE' + code = self.getRequestMessage(method) + reqParams = utf8_dict(self.log_args) + log_id = log_request(code, reqParams, None, method, + cherrypy.response.status) + save_request_log_id(log_id, task['id']) +

Is this result of the "if status != 202" added above?

Yes this is the "separate log" for requests that generate tasks (return 202).

...

return wok.template.render("Task", task)

@@ -454,7 +467,7 @@ class Collection(object): status = e.status raise finally:

...

- if method not in LOG_DISABLED_METHODS: + if method not in LOG_DISABLED_METHODS and status != 202: # log request

Same question I did before.

Same answer above :)

...

code = self.getRequestMessage(method) reqParams = utf8_dict(self.log_args, params) @@ -480,6 +493,15 @@ class AsyncCollection(Collection): args = self.model_args + [params] task = create(*args) cherrypy.response.status = 202

...

+ + # log request + method = 'POST' + code = self.getRequestMessage(method) + reqParams = utf8_dict(self.log_args, params) + log_id = log_request(code, reqParams, None, method, + cherrypy.response.status) + save_request_log_id(log_id, task['id'])

Is this result of the "if status != 202" added above?

Unless to save the log_id to the AsyncTask instance, shouldn't the log record be the same of the other HTTP return codes?

the task ID is only available here, and the log ID is generated when the message is logged. Here is the only place both are available.

...

+ return wok.template.render("Task", task)

-- Lucio Correia Software Engineer IBM LTC Brazil

On 01-09-2016 11:10, Aline Manera wrote:

Merge this patch with the other one where you edit the reqlogger.py

OK

On 08/31/2016 06:06 PM, Lucio Correia wrote:

...

Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/reqlogger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/wok/reqlogger.py b/src/wok/reqlogger.py index 2923d7c..7a95907 100644 --- a/src/wok/reqlogger.py +++ b/src/wok/reqlogger.py @@ -129,7 +129,7 @@ class RequestParser(object): code = error.get('code', '') params = error.get('params', {}) msg = WokMessage(code, params, plugin) - text += ' ' + msg.get_text(prepend_code=True, translate=True) + text += ' (%s)' % msg.get_text(prepend_code=True, translate=True)

return text

-- Lucio Correia Software Engineer IBM LTC Brazil

On 08/31/2016 06:06 PM, Lucio Correia wrote:

That log file is not intended anymore to be a log file itself.

Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/config.py.in | 4 ++++ src/wok/reqlogger.py | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/wok/config.py.in b/src/wok/config.py.in index cbe585c..f82aa0a 100644 --- a/src/wok/config.py.in +++ b/src/wok/config.py.in @@ -66,6 +66,10 @@ def get_object_store(): return os.path.join(paths.state_dir, 'objectstore')

+def get_data_dir(): + return paths.state_dir + +

You don't need a new function. You can access paths directly wherever you want.

def get_version(): return "-".join([__version__, __release__])

diff --git a/src/wok/reqlogger.py b/src/wok/reqlogger.py index 7a95907..92eff83 100644 --- a/src/wok/reqlogger.py +++ b/src/wok/reqlogger.py @@ -29,7 +29,7 @@ import uuid from cherrypy.process.plugins import BackgroundTask from tempfile import NamedTemporaryFile

-from wok.config import config, get_log_download_path +from wok.config import get_data_dir, get_log_download_path from wok.exception import InvalidParameter, OperationFailed from wok.message import WokMessage from wok.stringutils import ascii_dict @@ -60,7 +60,7 @@ TS_ZONE_FORMAT = "%Z" UNSAFE_REQUEST_PARAMETERS = ['password', 'passwd']

# Log handler setup -REQUEST_LOG_FILE = "wok-req.log" +REQUEST_LOG_FILE = "user-requests.data" WOK_REQUEST_LOGGER = "wok_request_logger"

# AsyncTask handling @@ -69,7 +69,7 @@ ASYNCTASK_REQUEST_METHOD = 'TASK'

class RequestLogger(object): def __init__(self): - log = os.path.join(config.get("logging", "log_dir"), REQUEST_LOG_FILE) + log = os.path.join(get_data_dir(), REQUEST_LOG_FILE) h = logging.handlers.WatchedFileHandler(log, 'a') h.setFormatter(logging.Formatter('%(message)s')) self.handler = h