search

aws / aws-sdk-php-zf2

ZF2 module for using the AWS SDK for PHP to interact with AWS services like S3, DynamoDB, SQS, EC2, etc.
http://aws.amazon.com/sdkforphp/
Apache License 2.0
103 stars 63 forks source link

Adds metadata for s3renameupload for use with Apigility #34

Closed wshafer closed 8 years ago

jeskew commented 8 years ago

This seems like configuration that would differ from application to application. Shouldn't this be supplied by consumers of this module?

wshafer commented 8 years ago

No this is metadata for Apigility only. The available options for the filters do not change from application to application. Those are specified and used in the filters themselves. The config provided here simply outlines what options the filter has not the values for those options.

Currently in Apigility the filter shows up in the GUI, but you can't specify the options for the filter since there's no definition provided in the module config. That's what this PR provides.

jeskew commented 8 years ago

This module does not just target apigility; it targets many different kinds of ZF2 applications. I'll need to do some research to make sure that this change does not change how anyone else consumes this module.

wshafer commented 8 years ago

:+1:

jeskew commented 8 years ago

I don't like the maintenance burden this introduces. Any option added by RenameUpload will need to be mirrored in the proposed change.

jeskew commented 8 years ago

@wshafer Is there any reason this shouldn't be added to the zend-filter component instead? The S3RenameUpload class just extends that module's Zend\Filter\File\RenameUpload class.

wshafer commented 8 years ago

@jeskew - It doesn't appear that the meta_data config follows the extends to get inheritance. If it did this filter would already have the options from the rename filter, since that's already defined. It seems to just pulls the data out of the config as is.

I think it's ok if we just close this PR since this is an Apigility only PR and doesn't have anything to do with ZF2 (per say) . I have added this config to my own app, I just thought I'd make it plug-in-play with Apigility to make it a bit easier for others.

Perhaps instead of adding it to the Module directy, maybe I can PR a note to the README.md file?

jeskew commented 8 years ago

I think that sounds reasonable. I've also seen separate modules that integrate ZF modules with Apigility, such as this one for Doctrine.