[Asterisk-code-review] res pjproject: Add ability to map pjproject log levels to A... (asterisk[13])

[George Joseph]

George Joseph has posted comments on this change.

Change subject: res_pjproject:  Add ability to map pjproject log levels to Asterisk log levels
......................................................................


Patch Set 5:

(3 comments)

https://gerrit.asterisk.org/#/c/2245/5/res/res_pjproject.c
File res/res_pjproject.c:

Line 52: 					<note><para>The id of this object, as well as its type, must be
       : 					'log_mappings' or it won't be found.</para></note>
> Any reason this is a requirement? Is this true for other types?
Yep.  If we don't know the exact id, we have to search (retrieve_fields) sorcery and the backend for it every time.  If it's predictable, we can use retrieve_by_id and that's cachable.


Line 149: 	RAII_VAR(struct log_mappings *, mappings, get_log_mappings(), ao2_cleanup);
        : 	unsigned char l;
        : 
        : 	if (!mappings) {
        : 		return __LOG_ERROR;
        : 	}
        : 
        : 	l = '0' + fmin(pj_level, 9);
        : 
        : 	if (strchr(mappings->asterisk_error, l)) {
        : 		return __LOG_ERROR;
        : 	} else if (strchr(mappings->asterisk_warning, l)) {
        : 		return __LOG_WARNING;
        : 	} else if (strchr(mappings->asterisk_notice, l)) {
        : 		return __LOG_NOTICE;
        : 	} else if (strchr(mappings->asterisk_verbose, l)) {
        : 		return __LOG_VERBOSE;
        : 	} else if (strchr(mappings->asterisk_debug, l)) {
        : 		return __LOG_DEBUG;
        : 	}
> Would caching these values make sense, so they don't have to be looked up e
Caching makes realtime difficult


Line 379: 	ast_sorcery_object_field_register(pjproject_sorcery, "log_mappings", "type", "", OPT_NOOP_T, 0, 0);
        : 	ast_sorcery_object_field_register(pjproject_sorcery, "log_mappings", "asterisk_debug", "", OPT_STRINGFIELD_T, 0, STRFLDSET(struct log_mappings, asterisk_debug));
        : 	ast_sorcery_object_field_register(pjproject_sorcery, "log_mappings", "asterisk_error", "",  OPT_STRINGFIELD_T, 0, STRFLDSET(struct log_mappings, asterisk_error));
        : 	ast_sorcery_object_field_register(pjproject_sorcery, "log_mappings", "asterisk_warning", "",  OPT_STRINGFIELD_T, 0, STRFLDSET(struct log_mappings, asterisk_warning));
        : 	ast_sorcery_object_field_register(pjproject_sorcery, "log_mappings", "asterisk_notice", "",  OPT_STRINGFIELD_T, 0, STRFLDSET(struct log_mappings, asterisk_notice));
        : 	ast_sorcery_object_field_register(pjproject_sorcery, "log_mappings", "asteris
> The defaults need to be added here so if a user doesn't specify that partic
Defaults only get applied if the file/section isn't found.  Otherwise a default setting could conflict with a specified one.  It's all or nothing.


-- 
To view, visit https://gerrit.asterisk.org/2245
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba7bb349c70397586889b8f45b8c3d6c6c8c3898
Gerrit-PatchSet: 5
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-HasComments: Yes