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

Decouple from Zend\* #4

Closed
asm89 opened this issue Jul 14, 2014 · 19 comments
Closed

Decouple from Zend\* #4

asm89 opened this issue Jul 14, 2014 · 19 comments

Comments

@asm89
Copy link

asm89 commented Jul 14, 2014

It would be nice if this library wouldn't have a hard dependency of Zend*.

@linaori
Copy link

linaori commented Nov 17, 2014

👍

1 similar comment
@nicoschoenmaker
Copy link

👍

@justinpage
Copy link

@lcf @asm89 @iltar @nicoschoenmaker We can decouple away from Zend's config if we follow the method constraints. This relies on the use of PHP's magic methods and the Countable, Iterator and ArrayAccess interfaces. I would love to write an object that does just this.

But would this be necessary just for the sake of not calling Zend? What is the exact purpose of de-coupling away from this specific config service? From the read me, I see two services involving Zend: config\config and \Zend\Di. Both are somewhat interchangeable with other services, if you wish to follow their constraints.

I guess to sum up my point, is the goal of decoupling from Zend is to make phystrix responsible for it's own implementation of accessing data and injecting dependencies? If we relied on, say, a Symfony implementation, would that be more fitting because of the sake of popularity?

Would love to hear some thoughts and opinions.

@linaori
Copy link

linaori commented Mar 15, 2015

I'm not too familiar with this code, but the most logical step would be to make a part of the code work stand alone and have bridges implement what ever is required to make it work for that framework. In Symfony this could be a bundle (or bridge), while in Zend this could be a Module (?).

@nicoschoenmaker
Copy link

First of all I have to say that I find this library really interesting. It would really solve a problem for me.

Ideally this package would not require a single config library. Every person in the world already has a config library, be it from Zend, Laravel, Symfony, whatever. Currently odesk/phystrix requires zend-config and zend-di, and based on that you also get zend-code, zend-eventmanager and zend-stdlib. So in total you get 5 additional dependencies.

I'm not familiar with the code, and how you depend on these libraries, but if you have a dependency that you don't want (or would prefer to have it as a suggest, rather then a require) here are some interesting reads

Ideally there should be one interface (or multiple) that anyone could implement to use their favourite config system. If this would exist, we would be happy to provide a Symfony related implementation.

@jasondt
Copy link

jasondt commented Mar 18, 2015

I think a case could be made for using Netflix/Prana (https://github.com/Netflix/Prana) to allow interfacing with Achaius for config, Eureka for load balancing and failover, and ribbon for rpc and load balancing... you can essentially run the full netflix stack for PHP now, so why not?

@sepn
Copy link

sepn commented Mar 18, 2015

@asm89 @nicoschoenmaker @iltar @Klvtz Obviously we could write custom DI and Config systems so we don't include Zend libraries, but I am not sure what we gain here. Regardless of how we handle configs, most projects will need to bridge the gap between how they handle configs and how they are passed into Phystrix. Internally, we use this library with Symfony2. @lcf will be open sourcing our Symfony bundle soon. I am sure others can write similar wrappers for their framework of choice.

@jasondt Prana looks great and I am glad Netflix released it. But I don't see how it solves this issue. We would replace a few PHP dependencies with an external process. Additionally, it wouldn't address DI. Although having Eureka/Ribbon are nice Hystrix/Phystrix solve a different problem and have no dependency on other components of the Netflix stack.

@lcf
Copy link
Contributor

lcf commented Mar 31, 2015

Phystrix bundle was open sourced: https://github.com/odesk/phystrix-bundle.

@nicoschoenmaker
Copy link

Perhaps something like upcoming PSR-11 could solve this? https://github.com/container-interop/fig-standards/blob/master/proposed/container.md

@lcf
Copy link
Contributor

lcf commented Jun 17, 2015

Thanks @nicoschoenmaker! We'll keep an eye on it

@jasondt
Copy link

jasondt commented Jun 21, 2015

@sepn I mentioned using Netflix Prana mainly to leverage Archaius (https://github.com/Netflix/archaius) as a distributed alternative for Zend Config. I went on to mention Eureka as service discovery is a key tenant of SOA and since we're embracing the Netflix stack already, why not continue the trend for our other needs?

@jasondt
Copy link

jasondt commented Jun 21, 2015

Basically, I'm glad we have Symfony in our tool box and I love what you guys are doing here to help make creating a robust distributed architecture easy with PHP, but we're still a ways away from having a standard drop-in solution like Java developers get with Spring Cloud.

@LegendOfGIT
Copy link

I managed to get rid off Zend\Config ;)

#29

@lcf
Copy link
Contributor

lcf commented Mar 28, 2018

@LegendOfGIT we didn't quite come to a conclusion on what is the problem that this would be solving. Can you elaborate on the reasoning behind the PR before we review it?

@LegendOfGIT
Copy link

@icf
The problem is, that devs are forced to use a bloated package (Zend\Config) just to gain a couple of values for phystrix from their config.

Furthermore devs can't use their config storage if it implements the ArrayAccess and is no instance of Zend\Config at the same time.
This restriction is simply unnecessary, since the basic functionalities of the php framework can be used instead.

@lcf
Copy link
Contributor

lcf commented Mar 29, 2018

@LegendOfGIT,

Since this will break BC and as with any change there are risks - I would just like to understand the specific improvements this brings.

  1. Regarding bloated - what does it mean exactly, what are the consequences for developers that we absolutely must reduce the size of this component? A specific scenario where this is needed or might be a blocker would be appreciated.

The PR introduces a bunch of new classes/interfaces and custom written logic to work around the fact the features available in Zend\Config are no longer available. Features such as merging configs and getting nested values. I believe it is better to reuse something thousands of developers have battle-tested, rather than invent a bicycle.

Btw, we would need to seriously check whether the solution with array_merge does the same job as \Zend\Config::merge and there are no caveats. As I remember there was something exactly about "merge" that made us go for Zend\Config.

  1. Regarding ArrayAccess: we not only use Zend\Config internally but also make it possible to use it for all developers who write commands - they have an instance of the config with all the features available to them inside their commands. It can be used for passing and working with any custom config. We do use that ourselves.

So I guess, before we decide to act on this, I need to understand the "bloating" problem that you mentioned and why it is critical to solve it.

(I'm @lcf, with an L :))

@LegendOfGIT
Copy link

@lcf
There is a reason why this issue is relevant over the years ;)

Devs don't want extra packages, as long as there is a good reason to require it.
The merge functionality and the override during runtime is covered with multiple unittests.
So there is no real potential risk.

Imho I haven't invented a bicycle.
I was using functions like array_merge which brought out of the box with the older versions of php framework.

@lcf
Copy link
Contributor

lcf commented Mar 29, 2018

Devs don't want extra packages, as long as there is a good reason to require it.

I agree with you. Reason being good or not is subjective, however. We feel like Zend\Config adds value - not only internally, but for command implementers as well - and it's better to reuse existing code than to write the same thing from scratch.

I'm going to close this issue now, to avoid further confusion. However it is possible we make use of your PR or parts of it when/if a major overhaul of the library is required and we decide to update the public contracts of the library.

@lcf lcf closed this as completed Mar 29, 2018
@linaori
Copy link

linaori commented Mar 30, 2018

@lcf it's quite simple, I don't need the Zend DI/Configuration just for this package, it should not be having a hard dependency to a framework. For me a dependency like this while I'm using Symfony, is simply a no-go. If everyone has these kind of dependencies, I'll end up with 4 different DI components and 5 config components from different frameworks in my application.

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

No branches or pull requests

8 participants