async-aws / aws

AWS SDK with readable code and async responses
https://async-aws.com
MIT License
436 stars 132 forks source link

AsyncAws V2 #1078

Open jderusse opened 3 years ago

jderusse commented 3 years ago

I was thinking about php 8.1 new features and that could be leverage in AsyncAws to simplify our API

 $dynamoDb = new DynamoDbClient();

 $result = $dynamoDb->describeTable(new DescribeTableInput([
     'TableName' => 'errors',
 ]));

-echo 'Item count: ' . $result->getTable()->getItemCount();
+echo 'Item count: ' . $result->table->itemCount;
-echo 'Read capacity: ' . $result->getTable()->getProvisionedThroughput()->getReadCapacityUnits() . "\n";
+echo 'Read capacity: ' . $result->table->provisionedThroughput->readCapacityUnits . "\n";

Migration path to maintain a v2 would be

  1. create a branch v2 in AsyncAws (PR to adds new Operation/client still target master)
  2. configure git splitter to split both master and v2 branches
  3. in v2 branch, update the generator to remove deprecated methods
  4. we now should tqg 2 versions for each service => time to automate this part?

Migration path for getter removal would be:

  1. in current version deprecate getters/setters and expose properties for people running PHP 8.1
  2. in v2 remove the getters/setters

Migration path for Enum switch would be:

  1. in current versions, adds Enum alongside the Class with consts and deprecates (for people running PHP 8.1) the old classes
  2. in current version properties (see above) use Enum, but getter cast the value to string (for BC) and setters/constructor converts the string to Enum
  3. in v2 remove the old Class (they should not be used anymore)

Migration path for paginated properties

  1. the getter will be replaced by a property containing a rewindable generator that contains all the logic.
Nyholm commented 3 years ago

Thank you.

Except for using the cool new features of PHP 8.1, what other benefit would this bring to the end user?

jderusse commented 3 years ago

Well.. except using properties instead of getters, and reals Enum, it brings nothing to the end users... :disappointed:

Nyholm commented 3 years ago

Hm. I agree with this changes. But there is no rush to do v2 just because these changes.

stof commented 1 year ago

Looking at the new generation SDKs of AWS based on their Smithy codegen tool, it looks like they intentionally use open enums for all their enums, saying that the SDK must not throw an error if it returns a new value for that enum (in case you haven't updated to the latest version of the SDK when they introduce new values). Given that PHP enums are closed enums, switching to real enums might not be a good idea (the implementation of open enums is actually what we have in 1.x: a string type with constants for known values).

Migration path for paginated properties

1. the getter will be replaced by a property containing a rewindable generator that contains all the logic.

Paginated methods have a $onlyCurrentPage argument right now. This cannot be reproduced with a property. So I'm not sure this switch is a good idea.

stof commented 1 year ago

Migration path for Enum switch would be:

1. in current versions, adds Enum alongside the Class with consts and deprecates (for people running PHP 8.1) the old classes

2. in current version properties (see above) use Enum, but getter cast the value to string (for BC) and setters/constructor converts the string to Enum

3. in v2 remove the old Class (they should not be used anymore)

As long as the getter returns the backing value, this does not allow projects to actually migrate to enums for any code using the getters.

stof commented 1 year ago

Note that our current phpdoc types don't actually reflect the fact that they are open enums, as they document return types as returning only one of the known constants. So static analysis tools will still think they are closed enums.