The httplug from php-http

Documentation Planning

This is an issue to discuss fundamental documentation topics.

Questions

where do we keep the documentation source
1. Separate documentation repository or
2. In doc/ folder of this repo
format
1. Markdown
2. reStructured text (with sphinx)?
3. other?
hosting
1. github pages (markdown or rst)
2. readthedocs.org (sphinx, lots of flexibility)
3. other?

Custom SPL exceptions

Should we make out SPL exceptions final?

The case came up: I created an UnexpectedValueException which extends the original SPL exception, which extends RuntimeException. However it does not extend our RuntimeException, which is a bit of inconsistency.

This is why I think making our SPL extensions final. They should be used as is. Other exceptions can extend the SPL exceptions and implement our interface.

Remove Configuration related methods from interface

IMO Configuration is an internal solution for passing details to the adapter and should not be part of the core interfaces

Plugins

Now that PSR-7 is immutable, event system should probably be exchanged with a middleware plugin system. (See explanation in #35)

Here is a list of plugins planned (new or migrate from Ivory):

Future of the project

Hey @php-http/owners, @dbu and @ddeboer,

For the last few months I've made some progress with this project. Sadly, @egeloen disappeared so we couldn't work together or discuss the phases of the project. I really appreciate the help of everyone who participated. I think this will be a great part of the PHP community once it gets stable. I am completely commited to the project, and I already have some feedback that other projects would like to rely on it.

As said, @egeloen does not really respond to any of my attempts to contact him. @dbu agreed to contact him (or at least try it). Until then, I would like to make progress with the project and reach at least an alpha phase, so other projects can start working with the interface. This does not mean that @egeloen is not part of the team anymore, but we have been waiting for four months now...

The current proposal of the interfaces is in the terminology branch. Currently exceptions are being refactored. After that I am going to merge the terminology branch.

My purpose is to reach alpha till the end of September.

Guzzle PSR7 implementation

https://github.com/guzzle/psr7

Supporting php7

With the upcoming release of php7 all libraries under php-http should support php7 IMO, WDYT ?

(The change will be to remove php7 from the allow_failure in travis file in all libraries)

Promises

Implementations

Currently we have our own Promise interface, which is fine. It seems that we won't have a Promise PSR anytime soon.

However, there are already awesome promise packages:

I remember that Guzzle used to use ReactPHP's promise implementation. Maybe @mtdowling could tell us why they created their own?

Anyway: would it make sense to rely on third party promise implementations instead of ours?

Possible ways:

Keep the interface in the contract and use wrappers around third-party promises
Drop our interface and rely on a third-party directly (which would require relying on an implementation in the contract)

Convenience methods

Currently we have two convenience methods: getResponse and getException. These are what make our interface a "special" promise implementation. As the above two implementations, ours claim to be an implementation of Promises/A+ standard. Although I see the point in these methods, I think we should probably get rid of them.

Here are a few reasons:

If a Promise PSR ever becomes real, we would still need to extend it. In this case we couldn't just accept any kind of promises to be used.
The same case, if we want to use a third-party implementation now.
It is not part of the original standard, but as I can see, returning the value in the wait method is.

I propose removing those method. However in that case we need a "standard" way of getting the value and deciding whether it is an exception, a response, or an invalid value. Checking the type is obvious, but might led to duplicated code. Maybe we could check how Guzzle does it.

PSR adapter implementation virtual package

Currently we have a virtual package for an adapter implementation. Packages could also provide a psr-adapter-implementation which allows packages to separately provide a PsrHttpAdapter and/or an HttpAdapter.

Tag alpha

Are there any more blockers before tagging alpha?

Even if documentation is not completely ready, if everyone is happy with the current state, we should freeze and tag it. We can move forward with documentation. Those, who want to play with it, can do it without documentation, as the code itself is documented enough in docblocks (I think).

Terminology

I am thinking about the following:

We could make adapter/client a set of interfaces which are recommended for client implementations. This way if an HTTP Client PSR is created once, we already have something.

The adapter naming is useful in the context of creating adapters wrapping clients. But for the long term, clients themselves should have a common interface which makes the adapter naming a bit vague.

I remember @jeromegamez mentioned something similar.

What do you think?

Separate interfaces for client, server and networking exceptions

As discussed in #10.

Separate message factory into a new package

I know that too much packages is bad......but it seems natural to have a separate package for message factories.

It makes the contract to be reusable for PSR-7 implementing libraries.

Introduce ConfigurableHttpAdapter

As mentioned in php-http/adapter-core#4 Configuration should be replaced with simple arrays. This allows us to create a simple extension of the HttpAdapter which lets setting global and request context options.

Communication

Hey guys,

I have been thinking about if we could improve the communication. Github issues are awesome, but we could do better. Since this is the first time for me to be part of such a large group (I usually did things alone so far), I only have a few guesses how it could be done in an organized way. If any of you have better ideas, I am open to them.

So we have github issues. Great, but sometimes direct communication would be better.

We have a gitter room, but let's admit: noone really wants to use yet another communication channel.

I was thinking about slack. In theory, it can be made public as well using slackin.

Other possibilities: email, skype, twitter.

So what do you think guys? What do you use at FOS/Made with Love?

Rename HttpAsyncClient to AsyncHttpClient

Name seems to be more logical this way.

Ping @joelwurtz @dbu

List of possible options

We need to have a list of possible values for the options parameter. These are general options which should work with ALL the clients.

Possible options:

timeout [0]
exceptions [false]: whether the user wants 4xx and 5xx responses to be converted to exceptions
base_uri [null]: from #36

Virtual packages

Currently we have adapter-implementation virtual package which could be renamed to client-implementation, however we now have three interfaces.

Possible virtual packages:

php-http/psr-client-implementation
php-http/methods-client-implementation
php-http/client-implementation

New name

The PHP HTTP client is an abstraction layer to separate general purpose libraries from concrete client implementations in order to make them move flexible. It is the successor of the Ivory HTTP adapter and we should not use Ivory anymore to keep it separate from all the other things under the Ivory name that do unrelated things.

Just calling it "php http" makes it impossible to find this thing in google, as almost website about web development has these keywords. We should have some additional name that makes it findable.

Ideas so far:

CPHC/APHC for common php http client / abstract php http client by @dbu
Klant by @hannesvdvreken
Raspberry by @Zarkonnen
Hunlet by @patkar
Tupac by @greg0ire
Straw by @Pierstoval
Httplug (and variations) by @sandermarechal
Phetch by @sandermarechal

HttpAdapter don't extend PsrHttpAdapter

I wonder if there should be an inheritance between the two interfaces.

HttpAdapterFactory

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?

Batch exceptions/responses

With the terminology branch merged, I only see a few questions we need to clear, this is one of them.

Currently parallel requests work like this: if the client supports it, send requests parallel, if not, emulate it by sending them one by one. In the latter case exception have to be caught and stored for later processing.

The return value of the parallel request function is currently an array of Response objects. The main problem with that: it is hard to identify which response belongs to which request. In an ideal case, array indicies match. The proposal here is to return an object which can return a response for a specific request. Internally we could use SplObjectStorage to achieve that.

In exceptional cases, currently a BatchException is used. The problem is the same as earlier: we cannot pair requests to exceptions. SplObjectStorage could help in this as well.

Another limitation is that currently only TransferExceptions are allowed to be stored in the batch exception. However it is possible that a request is rejected because of some sort of incompatibility: invalid method, etc. In these cases the underlying client might throw an exception which cannot be transformed to a TransferException, rather an InvalidArgumentException. So we either allow any exceptions to be added for a request (which implement our interface of course) or we require to convert that exception to a TransferException?

And a third problem: in case of any exceptions thrown, all the requests should still be processed. However, if we throw a batch exception, we won't get the successful requests. A solution for this: remove batch exception and use a combination of batch exception and response proposed earlier. This way we get all the results for requests (either response or exception) and don't block the processing.

Remove message interfaces

This is the opposite of php-http/adapter-core#3

I've been studying how the current interface model is built up and I wonder if we really need all those interfaces.

As I mentioned in the other issue, the implementations of these classes are internal. Furthermore, the request and the response are only including the MessageTemplate trait and a constructor, nothing more. So I wonder if we really need all those interfaces in the contract package.

Message is probably OK. MessageFactory and InternalRequest as well. But the other two interfaces? It seems to me that Request and Response are rather internal implementations which does not need their own interface.

Looking through the (contract) package there is only one place where these interfaces are used at all: exceptions. And even there, I would say we should typehint for PsrResponseInterface instead of the internal one. Nothing is used from the internal response object, so IMO we can safely widen the class dependency range.

I am going to provide a PoC PR to see if I can reduce the number of interfaces and find a way to avoid Interface suffix.

Drop options argument

In a previous PR we started to think with @dbu that we should drop the options argument. So far we could only found the timeout option which could be passed there. We would like to prevent that options is used for client specific things.

Actually timeout is something that might not be needed to be set for every request, but configured in the constructor.

So the current proposal is to drop the options parameter entirely.

Create common Exception interface, break exceptions into smaller parts

It is a common practice to introduce a generic Exception interface so that all exceptions thrown by this library can be caught in a single catch.

Also, as discussed on gitter, we should create more exceptions instead of one or two big ones.

Make adapters PSR-7 compliant

As discussed in php-http/guzzle6-adapter#1: there should only be only one adapter interface and that will take PSR-7 request objects and return PSR-7 response objects. So we need to:

remove the HttpAdapter
rename PsrHttpAdapter to HttpAdapter(?)
create a separate client package that will contain the convenience methods currently in HttpAdapter (get()/post() etc.).

If we follow this road, are the internal_separation still relevant? Or can we start work on this right away?

Move createInternalRequest to InternalRequestFactory

I am thinking about if we should move the internal request creation method into a separate interface.

Remove name from adapter

Name is used in some exceptions. But: is it really needed?

document when exception is thrown

as discussed in php-http/documentation#30, we seem to only throw an exception when there is no response object available.

Exception in this case is more when connection or transfer, parsing failed. Here it transform response who was correctly received but has a status code between 400 and 599 (the status code list where it throws exception can be changed by configuring the plugin)

i guess that is what we agreed on, but if that is correct, we should update the phpdoc of Client to say when exceptions are thrown (basically if no response is available due to errors in the networking, or if the client decides to throw an exception for other reasons... maybe we should just mention the error plugin to make this more concrete. would that make sense?)

Avoiding dependency hell

Currently each adapter of php-http will have his own repository, i'm afraid to see a big complexity if some days an interface need to be changed and all adapter will then have to be updated and match against the new version.

My proposal is to have all the code / issue / ... on one repository and make sub view (like symfony with its components), so a PR for updating a interface will also contain the modification for all corresponding adapter / client.

Remove version constants from PsrHttpAdapterInterface

I feel that they should go somewhere else...

Remove name from client, create adapter interface

By moving to a better terminology (according to #38) we should remove getName from the interface. However getName plays an important role in case of adapters, so I propose creating a simple adapter interface:

interface HttpAdapter
{
   public function getName();
}

naming of the virtual package

we currently say that adapters must provide php-http/adapter-implementation.

this should become php-http/httplug-implementation, right?

Implement remaining adapters

Make MultiException final

I can't really imagine a case where this type of exception should be extended.

Add gitter badge

Use phpspec for testing

As we discussed on Gitter, we should start to use phpspec+behat instead of phpunit where possible.

Base URI handling

Should we add a base_uri option? Alternatively, this could be done with a BaseUriSubscriber, which checks whether request URIs are relative, prefixes them with the base URI (cf. #35).

Consider Monorepository

Having gazillions of repositories for something that is essentially just an adapter seems overkill for maintenance and user adoption. See for example doctrine/cache, which has all the adapters in a single project.

Utility package

Currently there are a few things in the package which are not really part of the contract:

Body implementations
BatchRequest trait
Plugin to convert HTTP Error responses to exceptions (and the HttpErrorException class)

The plan is to move them to a utility package.

Make HTTP Client a PSR standard

This is not an immediate thing, but more of a discussion about the future:

liuggio suggested to propose the client as PSR. i think this is a viable idea, but will take more time. see also the discussion on PSR that liuggio referse to.

i think at this point we should just release what we have, as it will improve the situation and bring PSR-7 for generic HTTP Client consumers this year.

but we should try to get pinged/involved with the PSR discussion when they pick this topic up again. looking at the interfaces they throw around, i see there is little difference at the core between a server and a HTTP client: you have a request and want a response. whether the implementation produces the response itself or does a web request is more of an implementation detail. though the situation you use one or the other will often look different, and e.g. the kind of errors that can happen look different...

with the time just PSR-7 took, i think it will take a long time to get a PSR for this topic released and we should not wait, rather go with this abstraction and gather experience that can help build the PSR.

/cc @weierophinney, @liuggio

Conventional way to configure client

Httplug interfaces does not provide a way to configure client behaviour such as timeouts, SSL settings or other parameters that cannot be set in the RequestInterface.

For example. In dev environments we want our HTTP client to ignore SSL errors. But we can do this only when instantiating particular implementation. It has two disadvantages:

we can't use HttpClientDiscovery;
code, where the client is introduced through the interface, can't change these settings.

Review interfaces

Currently there are a view interfaces which are not closely part of the adapter contract:

Internal Request
Message Factory
Message Factory Aware
Parameterable Message

These are only used together with the core (especially the first one).

I wonder if we should move them somewhere else so the main contract only contains the minimal number of interfaces which are needed to create a custom implementation.

Base URI in message factory

Should it really be part of the interface?

Remove Interface suffix from interfaces

It is commonly believed it is also part of some PSR or recommendation. Actually it is part of the FIG's internal policy.

However I (and thank god, many other people) think it is just wrong to place an implementation detail in object naming conventions. Thus I propose to remove the interface suffix from interfaces.

Data and files in InternalRequest

I am not sure I completely understand what these two variables are for. Can't the to be merged somehow? Maybe introducing a FileContainer and if it is passed as data it should be used.

A StreamInterface and a files array cannot be passed together. Can a data and a files array be?

Contributor Code of Conduct

I know the Contributor Code of Conduct from FuelPHP. It was part of my general boilerplate, but I removed it for some reasons. In the last few months I was thinking about adding it back, but @philsturgeon gave me the last push by adding it to the League skeleton repo.

So I decided to add it to this project. It is a good guide next to our technical one.

datas -> data

I think using datas is incorrect and data is the correct form. Actually, data is already plural form. So I propose changing it to data everywhere. (Some BC break is also involved)

Cookie handling

@ddeboer mentioned that cookie handling should be possible from adapters.

In the Ivory adapter it was possible by using event dispatcher. I checked the Ivory and the Guzzle CookieJar implementation and they are really similar.

There are two ways to implement cookie handling:

If the underlying client supports it pass a cookie option
Force the use of an Event adapter which wraps the underlying adapter

Option 2 would be the general solution because not all clients might support cookies. Ultimately the underlying client (like guzzle) would do the same: serialize the cookie jar and place it in the request header.

Documentation

Currently we have a separate documentation repository. The main reason I separated it is that this way we can have one documentation for all the adapter-utility-messageFactory-helper thing. Should we keep that or separate documentation into their own repositories? (By that I mean documentation should go into the repository it actually documents)

Adapters probably doesn't need a separate documentation page (README is enough?) as configuring a client is relatively easy. Inject the Guzzle Client for example, done.

Projects that need documentation:

Adapter/Client
Message Factory
Plugins
Discovery

So, should we keep all these in one repository or put them into their own repo?

Downside would be VCS: documentation fixes also require tagging releases. This might be confusing for example in the contract: nothing changed (except the docs), but a new release is out.

php-http / httplug Goto Github PK

httplug's People

Contributors

Stargazers

Watchers

Forkers

httplug's Issues

Implementations

Convenience methods

Recommend Projects

Recommend Topics

Recommend Org