runtime parameter to control datasource plugin strictness and verbosity

[springmeyer]

As per #792 - all datasources should (be able) to throw if fields are requested that do not exist in the datasource. The reasoning behind this is that this condition is very hard to debug for stylesheet authors if datasources are silent.

But, we should allow this behavior to be disabled, so that these runtime checks can be bypassed for top speed or in production environments where errors are not desirable.

So, what about all datasources expecting option(s) to control whether error like this throw, print to std::clog, or are silent?

In the CSV plugin I've been using two options like:

quiet=bool, strict=bool

Or we could move to something like:

debug=1|2|3

Other ideas for naming?

[kunitoki]

We should try to merge efforts here to ease smashing bugs when developing. Other interesting stuff in #937 we should definately consider.

[kunitoki]

i've introduced the concept of log severity (much like we have in other logging libraries, especially for java and python).
this way we can use "info" "debug" "warning" "error" "fatal" "none" as available options, and if the wanted severity of the plugin is >= the currently logged severity than it will be logged otherwise not.

one example:

global severity = error

only mapnik::error() or error::fatal() will be printed, but mapnik::debug() or mapnik::warn() not.

another example:

global severity = debug
datasource severity = error

the datasource here is overriding the global severity, so only mapnik::error() or error::fatal() will be printed (for the datasource only, other parts of the library will still print mapnik::debug() in the logs).

this leads to another step: global log class enablers: you can enable or disable severities for all classes independently.
aka, we don't need specific datasource options, but instead you can enable logging and severities for each class.

we define a macro that takes an identifier

#define MAPNIK_DEBUG(s) mapnik::debug(#s)
#define MAPNIK_WARN(s) mapnik::warn(#s)
#define MAPNIK_ERROR(s) mapnik::error(#s)

this way you can do things like:

static void global_function_example()
{
    MAPNIK_DEBUG(global_function_example) << "This will be logged in debug";
}

class test_class
{
public:
    test_class()
    {
        MAPNIK_ERROR(test_class) << "This class will be logged as error";
    }
};

then from c++ you can do:

using mapnik::logger;

// setting global severity
logger::set_severity(logger::severity::none);

// per object severity overrides
logger::set_object_severity("global_function_example", logger::severity::debug); 
logger::set_object_severity("test_class", logger::severity::error);

or from python:

import mapnik
logger = mapnik.logger

# setting global severity
logger.set_severity(logger.severity.none)

# per object severity overrides
logger.set_object_severity("global_function_example", logger.severity.debug)
logger.set_object_severity("test_class", logger.severity.debug)

[springmeyer]

looks powerful. have you also considered allowing setting via an ENV (getenv) variable?

[kunitoki]

Where do you think we can place the getenv calls in order to be executed globally ? Calliong getenv everytime we hit a debug print may be too much...

[springmeyer]

just was pondering that at startup perhaps the severity default could be set based on ENV value, if it existed. After startup the only way to set it would be through the api.

[kunitoki]

where is the mapnik.so startup ? DllMain on windows and attribute((constructor)) init() in linux/solaris ?

[kunitoki]

I've started a check for format from getenv when the format itself is first parsed (but the check will be done everytime we output a string), check it at https://github.com/mapnik/mapnik/blob/master/src/debug.cpp#L88
don't know but it seems overkill to check for this everytime.

The same for enabling log, or setting global severity.

[springmeyer]

okay, no worries - scratch that idea of getenv!

[kunitoki]

Well, we could figure out a good way to handle this.

What we need to think is how to handle the plugin/library strictness.

[springmeyer]

@kunitoki - one issue with the MAPNIK_LOG_FORMAT. Having it passed on the compiler command line is looking problematic - it is a hard to escape string. It works for me with clang++ on osx, but breaks on windows (the compiler interprets the % poorly and I'm not sure how to escape). Ideally we could just force the user to manually override at https://github.com/mapnik/mapnik/blob/master/src/debug.cpp#L30? and not set in the CXXFLAGS? The other place this has broke things is when building node-mapnik, which uses mapnik-config to inherit CXXFLAGS and the node-mapnik build system is python and when the flags are passed through python things also break oddly.

[kunitoki]

yes this seems sensible. it's possible to set the format via API now (and bindings, so probably it's not needed anymore). the only thing could be the output that already do when you import the shared object from python for example, you cannot execute code before it's imported (but this isn't a problem anyway).

i will thrash setting the format from CXXFLAGS

[springmeyer]

okay, excellent. re: output just from importing the shared object - that will likely be lessened when we move to more lazy datasource creation - this is coming in the move_bind_logic_to_layer branch. But, also should not be a big deal - I like the idea of the api for settings.

[springmeyer]

pretty happy without how this is working now, closing.