PHP8, Guzzle 7 and new Endpoints #21
Conversation
There was a problem hiding this comment.
Looking good. Might have preferred a couple different PR's.
I'm abit confused by the PHP8 stuff, is it not more that you've added support for later phpunit version? (Without adding the change to composer.json? With the side effect of bumping the min requirements).
Its good that you've added php8 to the list as it should work with that now.
I'll need to test it with LXDMosaic and check it works, then I can merge it.
| * @param string $value | ||
| * @return string | ||
| */ | ||
| public static function studly($value) |
There was a problem hiding this comment.
Is this needed? PSR says we shouldn't have class names with "-" or "_" so it seems like an un-needed code path?
There was a problem hiding this comment.
The Class name is ...\Logs\ExecOutput - but to use it in the code you need to do something like that:
$client->containers->logs->exec_output->read
and I don't want to do ...->execOutput->read - thats looking aweful :) so it needs a possibility to come from exec_output to ExecOutput... studly is the best option for it...
finally id also like to add a Trait that contains the __get method - so it can be used everywhere - but for now it would be a to big refactoring...
|
to be honest: the most of all this stuff comes directly from ashleyhoods repo... php8 was added there - and all the TestCase-Stuff is also related to PHP8 - I think it is because with php8 you automatically use a newer version of phpunit and then use different classes... Thats also the reason why it is only one PR - i just took all from there into mine and made it fit to yours... and with ashleys repo i'm using it since about half a year now with php8.1 - so yes I'm quite sure that it will work... but now with a newer lxd version I came into the situation that I need this execOutput-Endpoint and it does make more sense with your repo... |
This PR introduces support for PHP8 and Guzzle 7 adapter
Additionally it creates a new Endpoint ->logs->exec_output->
this is necessary because execute-commands that use "record"-option of output save the logs in dedicated record-output logs now. in former LxD-Versions this was all contained in logs-endpoint. But in meanwhile it is a dedicated endpoint: https://documentation.ubuntu.com/lxd/en/latest/api/#/instances/instance_exec-outputs_get
Until last year i contributed to the original version of ashleyhood - the PHP8 and Guzzle 7 topic was there already integrated. But since it doesn't support new endpoints and seems to be stuck on the old state, I decided to switch the fork. It would be great, if your version can do the upgrades