-
Notifications
You must be signed in to change notification settings - Fork 11
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
Nahrazení SoapClient za nějakou rozumnější abstrakci #97
Comments
Nerad bych kompletní výměnu, ale preferoval bych umožnit alternativu. Přidal bych do
Souhlasím, rozhodně bych se vyhnul jakékoliv ne-mainstreamove knihovne. Pokud to licence umožní, tak můžeme třeba část jiné knihovny vyloženě převzít. Také bych se zamyslel nakolik se vázat přímo na Guzzle a nakolik využít PSR-7/PSR-17/PSR-18. |
Určitě IMO dává smysl mít vazbu spíše na PSR-7 klienta. Spíš jsem tím Guzzlem myslel to, že IMO by se hodilo přidat závislost na nějakém konkrétním klientu, ať uživateli stačí přidat závislost na skautis knihovně a nemusí hledat ještě nějakou implementaci HTTP klienta. Ale i varianta s doinstalováním je asi ok. Náhradu jsem navrhoval vesměs kvůli tomu, že by nebylo potřeba udržovat dvě alternativní řešení. Pokud totiž nebude nějaký výrazný rozdíl v performance, tak IMO nedává moc smysl mít dvě různé implementace. Zvlášť pokud jedna z nich bude mít podmnožinu featur. |
Jinak i převzetí části knihovny asi dává smysl. Třeba ta, co jsem posílal, je pod MIT licencí, což by neměl být problém. |
Ještě teda koukám, že PSR-18 asi není pro naše účely příliš použitelné - jedná se pouze o sync API. |
Tim ale uzivateli omezime verze HTTP klienta na ty ktere podporuje Skautis. A zaroven tim omezime verzi PHP na ty ktere podporuje ta knihovna a jeji zavislosti.
Od toho aby nemusel hledat je
Tady mi slo o SOAP protokol, kdyby byl nejaky problem s nasi implementaci, mohl by uzivatel fall-backnout na SoapClient. Pokud bychom chteli |
Já jsem 100% pro interoperabilitu, ale AFAIK teď prostě žádný standard pro asynchronní HTTP klienty není.
Tohle nevnímám jako problém. Ve chvíli, kdy vyjde nová verze major této knihovny, to povede k nutnosti přidání podpory do tohoto balíčku. Na druhou stranu teď vychází Guzzle 7. Guzzle 6 tu byl 5 let a zatím je stále udržovaný. Jeden PR za 5 let IMO není tak velký overhead. Ohledně podporované verze PHP - jedná se o udržovanou knihovnu a troufnu si tvrdit, že na nové PHP verze bude připravena dříve než my. Značná část projektů, které Nehledě na to, že je naprosto ok dále používat verzi
S ponecháním SoapClient varianty ve výsledku nemám problém. Jak píšu, IMO je to zbytečná práce navíc, ale na druhou stranu to řešení se Další varianta je, že bych to naimplementoval jako samostatný balíček a pokud se to osvědčí, tak můžeme přidat přímo do |
Pravda.
Guzzle 7 ma zavilslost
Takze bychom to vytvorili na tu novou verzi, i kdyz v bete?
Spis bych nechal SoapClient jako default, Guzzle 7 jako dev dependency + suggest. A v
Pravda, to je ted potreba vymyslet. Napadji me tyto 3 moznosti: A) B) C) |
Tady bych osobně fakt upřednostnil samostatný balíček, protože:
*Pominu to, že nic jako optional dependency v pravém slova smyslu neexistuje, protože pokud přidáme implementaci proti Guzzle 6, tak implicitně závisíme na konkrétním API, což je IMO výrazně problematičtější varianta než explicitní závislost. |
Ok, tedy jaky bude dalsi postup? Dava mi smysl toto:
|
A je opravdu potřeba si na sebe šít bič v podobě podpory 2 implementací? Jakože co to přidává oproti možnosti tam prostě hodit guzzle? Google API client to tak myslím taky má. Navíc verze 3 stejně zvedá nejnižší podporovanou verzi PHP, takže nějaký odkaz na možné nekompatibility mi přijde lichý, protože to stejně uživatelé budou muset řešit. Za sebe async API vítám, myslím, že by to měl být výchozí způsob práce s knihovnou. |
Nejde jen o verzi PHP, ale i verze knihoven. Tech na kterych zavisi tahle knihovna a jejich zavislostech a tak dale. Tedy pokud by Skautis 3.x pozadoval Guzzle 7, nesel by pouzit v projektu ktery pozaduje Guzzle 6. Napriklad tebou zmineny Google API client ktery pozaduje Pricemz Guzzle 6 funguje i na PHP 7.4 tak nikoho prilis netlaci upgradovat. Dalsi priklad jak uz jsem psal drive tak Guzzle 7 dev verze vyzaduje
AFAIK ten implementuje primo HTTP API, nema imeplmentaci SOAP protokolu. Pro spolehlive pouziti by bylo nad Guzzle potreba implementovat SOAP 1.2 part 1, a WSDL 2.0 part 1 a mozna i SOAP 1.2 part 2 a WSDL 2.0 part 2. Pokud v tom udelame chybu, uzivatel ma problem. PHP nabizi leta otestavanou implementaci pro WSDL/SOAP, dostupnou pro vsechny pouzivane verze PHP. Tedy neomezuje pouzite knihovny ani verze PHP.
Jedine co by bylo potreba delat duplikatne je implementace |
No, Guzzle 7 není myslím zatím moc stable, takže s ničím výrazým by tam s šestkou konflikt nebyl... V budoucnu podpora 6 i 7 nevím, jak moc je reálná... Implementovat SOAP nad Guzzlem by se pro jeho podporu muselo tak, jako tak, ať už bude jedinou možností, nebo tam bude i ten SoapClient. Ale pokud to je opravdu tak jednoduché, jak píšeš, tak to pro vývoj není asi problém. Spíš se bojím, aby to nebylo matoucí pro uživatele, že mají na výběr 2 možnosti, jak tu knihovnu použít... |
Zatím bych jen přidal metodu do |
Rád bych navrhl pro verzi 3.0 výměnu Soap klienta. Je to něco, co minimálně ve svých projektech mám v plánu dělat, a tedy nabízím implementaci přímo tady. Rád bych využil buď Guzzle/PSR-7 (osobně mi přijde z uživatelského pohledu jednodušší mít závislost přímo na Guzzle), nebo např. meng-tian/async-soap-guzzle
Motivace
HistoryMiddleware
apod). Kromě toho některé části ani nejdou namockovat (SoapClient při vytvoření instance stahuje wsdl soubor, což není možné nijak přetížit).Nemám problém to připravit a poslat PR, ale zajímal by mě ohlas na podobnou změnu.
The text was updated successfully, but these errors were encountered: