1up-lab / OneupFlysystemBundle

A Flysystem integration for your Symfony projects.
MIT License
630 stars 118 forks source link

Features/env vars #228

Closed fullbl closed 1 year ago

fullbl commented 3 years ago

Resolves (#207 / #156) moving service binding into compiler pass, where env vars are already solved.

bytehead commented 3 years ago

Thank you, really nice! 🎉 Ignore my last comment (deleted) :) I'll check by the evening!

bytehead commented 3 years ago

Can you solve the merge conflicts or should I?

fullbl commented 3 years ago

Too bad it seems unfixable now: https://github.com/symfony/symfony/issues/20850. Tomorrow I'll take a closer look

fullbl commented 3 years ago

Uhm... It seems that env var are processed later in the Symfony building up, so, since we are working with the original League classes, I think it's not possible to get the right adapter from a var, unless we offer the customers a custom class that extends the filesystems and then set the adapter at construction time. Otherwise I see no methods to allow env vars in adapter definition. The only thing I can think of, would be to extend the adapters and use something hacky like trying to return a child class from a getter and set the real adapter as "returnClone" (https://symfony.com/doc/current/service_container/calls.html), but it looks a bit strange... What do you think of? Suggestions?

VincentLanglet commented 3 years ago

If it's not possible, or easily possible, what's about delaying it to another major in order to release the support of league/flysystem-* packages to ^2.0 in https://github.com/1up-lab/OneupFlysystemBundle/pull/217

bytehead commented 3 years ago

Thank you @fullbl for your research. I think I'll publish 4.0 without that feature. We can still add it in a new minor 😎 what do you think?

bytehead commented 3 years ago

@VincentLanglet I'm with you there 🙃

fullbl commented 3 years ago

I agree, it doesn't look like a simple quest!

bytehead commented 3 years ago

I'll let this PR open until there is a solution. I've tagged a RC1 for now: https://packagist.org/packages/oneup/flysystem-bundle#4.0.0-RC1 - feedback welcome :)

bytehead commented 3 years ago

Just for your information: 4.0.0 is out! 🎉

bytehead commented 1 year ago

See #277 for follow up