Problem/Motivation
Hi, thanks for the module! I'm maintaining another module that builds on this one (#2929260: Amazon Polly). When using phpstan to lint the code I came up against an issue where this module's docs weren't entirely accurate (eg. sometimes the doc mentions it might return false on failure, but doesn't indicate that in the actual @return).
Steps to reproduce
Run phpstan at level 3.
Proposed resolution
Fix phpstan level 3 issues and set the variable _PHPSTAN_LEVEL to 3 in .gitlab-ci.yml. We need to wait for #3609096: Error when using AWS profiles with an assumed role and STS AssumeRole fails to land to do that. Also ignore the errors that are fixed by that issue:
------ ------------------------------------------------------------------------------
Line src/Entity/Profile.php
------ ------------------------------------------------------------------------------
347 Caught class Drupal\aws\Entity\Exception not found.
🪪 class.notFound
348 Call to method getMessage() on an unknown class Drupal\aws\Entity\Exception.
🪪 class.notFound
------ ------------------------------------------------------------------------------
Remaining tasks
There's a remaining issue: Method Drupal\aws\Entity\Form\ProfileDeleteConfirmForm::getDescription() should return Drupal\Core\StringTranslation\TranslatableMarkup but returns string. Currently it just returns the empty string (which works for now, but breaks the core interface contract and could feasibly stop working without warning in a future release). I noticed that the following code block used to exist in 23148a4cfe13fae2fddd49fe66bdaf65de795881:
/**
* {@inheritdoc}
* @todo
*/
public function getDescription() {
$plugin_manager = \Drupal::service('plugin.manager.aws_service');
$services = $plugin_manager->getDefinitions();
foreach ($services as $service) {
$config = $this->configFactory->get('aws.' . $service['id'] . '.settings');
$profile_id = $config->get('profile');
if ($profile_id === $this->entity->id()) {
$affected_services[] = $profile_id;
}
}
return $this->t('@services', ['@services' => $affected_services]);
// Should return list of services that will be affected.
}
Should that be reinstated? Do we know why it was removed?
User interface changes
API changes
Data model changes
Issue fork aws-3609514
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
andyf commentedHere's a first go at that.
There's one point where I change the functional behavior:
\Drupal\aws\Entity\Form\ProfileForm::save()now returns the correct status.There's another point where it looks like I change the functional behavior, but it's actually the same: changing
AssumeRoletoassumeRolein\Drupal\aws\Entity\Profile::getTemporaryCredentials(). This matches the method names declared in the source via@method. It's a safe change:\Aws\AwsClientTrait::__call()calls\Aws\AwsClient::getCommand()which executesucfirst($name)on the method name before calling it - so we end up dispatching the same call in the end.Comment #4
andyf commentedComment #5
andyf commentedActually, rather than providing our own phpstan.neon, we can just set the variable _PHPSTAN_LEVEL. Updating IS and MR. Postponing on #3609096: Error when using AWS profiles with an assumed role and STS AssumeRole fails.