Skip to content
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

Multi handler support #19

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jirikuncar
Copy link
Contributor

@jirikuncar jirikuncar commented May 17, 2016

addresses #18

cc @ibnesayeed

* Adds support for any Werkzeug (like) cache backend.  (closes mementoweb#15)

* Adds new configuration option `CACHE_BACKEND`. All options in
  INI file `[cache]` section are passed to the cache constructor.

Signed-off-by: Jiri Kuncar <[email protected]>
if ':' in section_name:
self['HANDLERS'][section_name[len('handler:'):]] = section
else:
self.update(section)
Copy link
Contributor

Choose a reason for hiding this comment

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

What if we organize the configs of both types of handlers (named and anonymous) on the same level (i.e., under self['HANDLERS'])? One option is to use a hard coded placeholder handler name for the anonymous handler such as DEFAULT or ANONYMOUS, but this will add special cases in the application.py file and will limit the use of these reserved keywords as handler names. However, there is a cleaner option to use None (not the string "None") as the handler name that will remove special case handling while still organizing all the handlers at the same depth. Here is one possible way to write this (note that I have also added code to deal with the case where a section is names as handler: i.e., ending with a colon, but nothing after that.):

        self.setdefault('HANDLERS', {})
        for section_name in conf.sections():
            parts = section_name.rstrip(':').split(':', 1)
            if parts[0] == 'handler':
                section = build_handler(section_name)
                handler_name = None
                if len(parts) > 1:
                    handler_name = parts[1]
                self['HANDLERS'][handler_name] = section

This will also remove the following line from application.py file:

        self.register_handler(None, self.config)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had it this way initially, but I changed it to keep the defaults on "global" level, which are easier to override.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have improved the config handling. The "old" config values are used as defaults for all handlers. One can still keep default values and override the "default" (None) hander:

HANDLERS = {None: {'USE_TIMEMAP': False}}

or

[handler]
use_timemap = False

[handler:]  # see the "trick" with :
use_timemap = True

[handler:simple]
# falls back to use_timemap = False

* Adds configurable multi-handler support via `[handler:<name>]`
  sections in INI file or `HANDLER` key in config.  (closes mementoweb#18)

Signed-off-by: Jiri Kuncar <[email protected]>
@jirikuncar jirikuncar changed the title WIP Multi handler support Multi handler support May 18, 2016
@jirikuncar jirikuncar mentioned this pull request May 19, 2016
@ibnesayeed
Copy link
Contributor

@jirikuncar would you mind rebasing this PR to resolve the conflict and verify that it is still working? I am not sure whether it is still being considered in this repo, but I would like to use this feature elsewhere.

@jirikuncar
Copy link
Contributor Author

@ibnesayeed I have granted access to maintainers to update the PR. Feel free to rebase the code.

@ibnesayeed
Copy link
Contributor

Thanks for the help @jirikuncar. I tried merging like this the other day, but tests were failing after doing so. Also, I am not a maintainer of this repo. While I am in support of bringing multi-handler support to it, people at LANL feel differently. I am not sure if their opinion has changed since our last conversation on this topic. So, I asked for your help for my use of this tool, not for the sake of getting it merged here. I really appreciate your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants