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

HttpAdapterFactory #25

Closed
ddeboer opened this issue May 24, 2015 · 9 comments
Closed

HttpAdapterFactory #25

ddeboer opened this issue May 24, 2015 · 9 comments

Comments

@ddeboer
Copy link
Contributor

ddeboer commented May 24, 2015

The egeloen library had an HttpAdapterFactory that tries to automatically instantiate an HTTP adapter based on the HTTP client library that is installed. Should we create an php-http/adapter-factory (or adapter-guesser?) package, too?

@sagikazarmark
Copy link
Member

Currently our idea is that adapters provide php-http/adapter-implementation virtual package which means requiring that should explicitly declare which http adapter you use. We haven't decided yet how we should proceed about adapter guessing. Maybe we could create a package which requires all adapter implementations provided by us and we could implement a guesser there.

@sagikazarmark
Copy link
Member

Since the Http Adapter has been separated into actual Adapter and Client, this functionality should go into the client IMO.

@ddeboer
Copy link
Contributor Author

ddeboer commented Jun 2, 2015

Why no AdapterGuesser or AdapterFactory in adapter? That way other libs that require an HTTP client could just depend on adapter an use its AdapterGuesser to retrieve an adapter.

I don’t think this is top-priority though.

@sagikazarmark
Copy link
Member

I would not add that into the contract. It is not as stable as the exceptions for example. Either in the adapter client or in a separate package.

It is rather a hack than a real solution. In an ideal case, all dependencies should always be injected instead of auto setup.

@ddeboer
Copy link
Contributor Author

ddeboer commented Jun 2, 2015

How would you handle a case like FriendsOfSymfony/FOSHttpCache#173 (diff)?

@sagikazarmark
Copy link
Member

Yeah, I agree the guesser could be useful in that case. Do you think putting it in a separate package is a bad idea? (Because putting one class in a separate package seems to be a bit odd)

The case I would like to avoid is to version the guesser together with the adapter, because as new adapters are created, they must be added to the guesser (which makes the adapter pretty unstable).

One thing I can think of is adding an empty guesser to the adapter contract and somehow the installed adapters could register themselves during autoloading. But this also sounds weird.

@sagikazarmark
Copy link
Member

Maybe message-factory-guesser and adapter-guesser could moved into one package.

@sagikazarmark
Copy link
Member

@ddeboer https://github.com/php-http/guesser

Does this solve your issue?

@ddeboer
Copy link
Contributor Author

ddeboer commented Jun 2, 2015

It definitely does!

@ddeboer ddeboer closed this as completed Jun 2, 2015
Nyholm pushed a commit to Nyholm/httplug that referenced this issue Dec 26, 2019
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

2 participants