csarrazi / CsaGuzzleBundle

A bundle integrating Guzzle >=4.0 in Symfony
250 stars 77 forks source link

[Broken] Missing setPublic(true) on dynamic services #217

Closed cordoval closed 6 years ago

cordoval commented 6 years ago

i am getting a:

The "csa_guzzle.client.print_pdf" service or alias has been removed or inlined when the container was compiled. You should either make it public, or stop using the container directly and use dependency injection instead.

Because we missed the setPublic true on the extension or factory

using

        "csa/guzzle-bundle": "~3.0.0",
        "csa/guzzle-cache-middleware": "~1.0.0",
        "csa/guzzle-history-middleware": "~1.0.0",
        "csa/guzzle-stopwatch-middleware": "~1.0.0",

and

csa_guzzle:
    profiler: false # because it is too dependent on asset()
    logger: "%kernel.debug%"
    clients:
        print_pdf:
            config:
                lazy: true
                base_uri: "%printing_address%"
                headers:
                    Content-Type: 'application/json'
                cert: "%ssl_pem_path%/cert.pem"
                verify: false
cordoval commented 6 years ago

trying to address here https://github.com/csarrazi/CsaGuzzleBundle/pull/218

csarrazi commented 6 years ago

Ping @dunglas are there any ways to do that using Autowiring, without using a public service?

dunglas commented 6 years ago

@csarrazi if this service is used somewhere, it will not be removed. If it is not used, it should be removed. If you still want to be able to use $container->get(), you need to make the service public (as it was before Symfony 4).

cordoval commented 6 years ago

autowiring has nothing to do with this On Tue, Dec 12, 2017 at 4:46 AM Kévin Dunglas notifications@github.com wrote:

@csarrazi https://github.com/csarrazi if this service is used somewhere, it will not be removed. If it is not used, it should be removed. If you still want to be able to use $container->get(), you need to make the service public (as it was before Symfony 4).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/csarrazi/CsaGuzzleBundle/issues/217#issuecomment-350999314, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUCpxdsV1DrrGVoPRLXIXp-xkJ-iXqhks5s_kuFgaJpZM4Q9pDT .

csarrazi commented 6 years ago

Sorry, what I meant regarding auto wiring was basically whether we had a way to make the bundle work with auto wiring. Currently, you can not inject automatically a client in a controller or another service.

Actually, let me close this as I have already merged your PR. I'll create a separate issue for questions regarding auto wiring to @dunglas :)

By the way, @cordoval, pull requests are accepted even without any linked issue. You don't need to open an issue before creating a PR :)

cordoval commented 6 years ago

i know but i work in a flow on autopilot he he

i honestly dislike autowiring because you dont gain anything special other than magic

for apps maybe autowiring is for newbies but bundles should imo not necessarily autowire

csarrazi commented 6 years ago

Indeed. However people relying on auto wiring in their applications should still be able to use the bundle. From what I could see, you can not rely on auto wiring, given the way the bundle is defined.

dunglas commented 6 years ago

They'll need to the $ syntax:

_defaults:
  autowire: true

Foo\Bar: {$client: 'guzzle_service'}