ScriptFUSION / Porter

:lipstick: Durable and asynchronous data imports for consuming data at scale and publishing testable SDKs.
GNU Lesser General Public License v3.0
611 stars 24 forks source link

ConnectorOptions is a bad design #62

Closed Bilge closed 5 years ago

Bilge commented 5 years ago

At first glance, one would think tying options to a connector would create concurrency issues where two requests could set different options on the connector at the same time. Due to cloning, this is not an issue, however the problems with ConnectorOptions reach further than just potential concurrency issues. Since connectors may be decorated, finding the options you need to modify often means traversing the stack of connectors, but it's cumbersome and error prone to do this, by traversing the stack of connectors from ImportConnector down.

We cannot simply remove connector options and let implementations do as they please because the cache needs knowledge of the particular options exported by the connector in order to determine whether two requests are identical and thus the cache may be reused.

We propose changing the signature of fetch(string) to fetch(object), where object is some implementation-defined object that encapsulates both the original source string plus the connector options. In this way, everything needed to define the request is passed through all connectors in the stack and can be inspected or modified as needs be when it passes through. This also precludes the need to clone the connector (and its options), which makes implementations much easier and cleaner.

This change would be a BC break, and moreover, the signature is less convenient than simply passing a string, which can be sufficient for HTTP GET requests and some others. It is a consideration that we may support object|string, however this does complicate the interface and make it more taxing to implement.

Rather than just fetch(object) where object is literally typed to object, which is unsupported in PHP 7.1 anyway, we should probably have a Source interface that specifies toArray and serializes all configurable options as an array, for use with caching.