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

Kelp: allow multiple instances of an app #62

Closed
wants to merge 1 commit into from
Closed

Kelp: allow multiple instances of an app #62

wants to merge 1 commit into from

Conversation

aeruder
Copy link
Contributor

@aeruder aeruder commented Jan 22, 2019

Kelp generally loads modules, loads config, etc. all in the context of
a specific instance, but the entire module system is setup to load
class-wide methods.

Even a very simple program like the following dies:

use Kelp;

Kelp->new;
Kelp->new;
# Redefining of Kelp::config_hash not allowed at Kelp.pm line 105.

This means you couldn't, for example, load two instances of a Kelp app
with different configs (at least not without creating subclasses).

This commit moves registered methods to a per-instance hash and
overrides AUTOLOAD and can to hook them up. This change should
largely be backwards compatible unless class methods were being loaded
with the module system.

Kelp generally loads modules, loads config, etc. all in the context of
a specific instance, but the entire module system is setup to load
class-wide methods.

Even a very simple program like the following dies:

    use Kelp;

    Kelp->new;
    Kelp->new;
    # Redefining of Kelp::config_hash not allowed at Kelp.pm line 105.

This means you couldn't, for example, load two instances of a Kelp app
with different configs (at least not without creating subclasses).

This commit moves registered methods to a per-instance hash and
overrides `AUTOLOAD` and `can` to hook them up.  This change should
largely be backwards compatible unless class methods were being loaded
with the module system.
@aeruder
Copy link
Contributor Author

aeruder commented Jan 22, 2019

This changes how registered methods work pretty significantly - there are a couple other approaches too, but thought I'd start with this to open some discussion.

In my application, I'm using Kelp to help with mocking of external services in tests - in some situations I'd like to instantiate two copies of the Kelp microservice with different configuration - but it turns out that right now you just can't ever instantiate a second Kelp app at all.

@bbrtj
Copy link
Member

bbrtj commented Apr 15, 2020

Another reason for this change is the ease of access to autoloaded subs and replacing them without symbol table manipulation and turning off strictness.

I've lately created a module to allow changing the methods introduced by the modules: https://github.com/brtastic/perl-kelpx-hooks/blob/master/lib/KelpX/Hooks.pm#L26
This is a bit messy but it works, it has to replace subs with other subs just before build() is invoked. Would be much, much easier to just replace a record in registered_methods hash, probably even no need for my module other than syntactic sugar / default way of doing that.

Of course, it'll be no longer possible to say MyKelpApp->json anywhere to get a JSON decoder for example, so ultimately the change can break some poorly-written code

@sgnix
Copy link
Collaborator

sgnix commented Jan 16, 2021

Implemented in a more backwards compatible way in #71

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.

3 participants