PS: We're using this on production 👍
Also a good candidate for v2.0

ping @rmccue @joehoyle

@tristanbes the endpoint can be configured with the following:

// Filter S3 Uploads params.
add_filter( 's3_uploads_s3_client_params', function ( $params ) {
	$params['endpoint'] = 'https://your.endpoint.com;
	$params['use_path_style_endpoint'] = true;
	$params['debug'] = false; // Set to true if uploads are failing.
	return $params;
} );

This is the approach we used on a Minio extension for Chassis to help with testing this plugin locally.

I'll add this filter to the readme but I don't think this PR will be merged I'm afraid - there are others for the same functionality in #208 and #268

We know but:

1/ Why add an ugly filter to 150+ of our WordPress when a few lines can enable this behaviour in a clean way inside your plugin ?

2/ This feature has been tested and documented in this pull request

3/ It's not breaking the existing setup, nor introduce a BC break. It's just for those who needs it who can use it....

Please don't make us use a fork of your plugin :/

The main reason is to avoid adding another constant where it isn't strictly necessary. Constants are always global so it makes it harder to have multiple instances of S3 Uploads - some use cases we've seen involve sharding uploads to different buckets on multisite for example so using the filter is the most flexible solution.

The scope of this plugin is really just to support AWS S3 as it's a core part of our own infrastructure, support for other providers is a happy coincidence.

It can be made a bit cleaner by adding the filter in your own separate plugin that can be added to those sites (maybe as a mu-plugin) that then uses your constant to set the value.

It's not a happy coincidence, it's because they work to make themselves compliant with the S3 API.

Also, it's something that is supported inside AwsClient :https://github.com/aws/aws-sdk-php/blob/master/src/AwsClient.php#L94

Your point is basically the same like "we don't use the region feature, so we don't provide a way to configure them and we harcoded it for Arizona region inside our plugin"

😞

Fair points. I'll re-raise the issue internally but last time I did it resulted in just adding the client params filter so we'll see :/

For what it's worth in my own experience setting the endpoint alone wasn't enough and I had to configure other params like path style to make it work so adding the constant isn't necessarily a silver bullet.

Your point is basically the same like "we don't use the region feature, so we don't provide a way to configure them and we harcoded it for Arizona region inside our plugin"

This is basically our support policy. If we add features that we're not using, it's entirely possible that we'll break them in the future without noticing. Tests do help with this, but they can increase the effort needed for us to introduce new features in the future. Adding features increases our support and maintenance burdens, even if we don't have to write the code in the first place.

Given that the functionality here is already possible in a more generic way, I'm not sure what we really gain by adding a constant. The code in #296 (comment) basically achieves the same thing (you'll also need to filter s3_uploads_bucket_url for the same behaviour as in the PR).

1/ Why add an ugly filter to 150+ of our WordPress when a few lines can enable this behaviour in a clean way inside your plugin ?

You'll need to define the constant on your sites anyway, so I'm not sure what you mean here. The difference between a constant and a filter isn't that large.

With that said, I'm not going to say it's a definite no here, as this functionality may be useful for our purposes as well (for example, running Minio for testing). We're continuing to discuss this internally.

Hi, is there any status update on this matter ?

FWIW DigitalOcean spaces is also S3 compatible - this feature seems like a good idea

This is basically our support policy. If we add features that we're not using, it's entirely possible that we'll break them in the future without noticing. Tests do help with this, but they can increase the effort needed for us to introduce new features in the future

Well, even if my pull request have been used in production and worked like a charm, that's sure it's better to write tests for this.

But how? I wasn't even able to run my own tests because PHPUnit is not installed locally, and even if you use PHPUnit globally, all (except one 😮) of the tests are failing... And it's also happens on every opened PR, on Travis... come on 😞

Also, since d93f035 that document how to use filter s3_uploads_s3_client_params to configure the endpoint for uploads, maybe we don't need this pull request anymore.

And it's true that:

add_filter( 's3_uploads_s3_client_params', function ( $params ) {
	$params['endpoint'] = 'https://your.endpoint.com';
    // ...
	return $params;
} );

it 100 times more flexible than this:

define('S3_UPLOADS_BUCKET_ENDPOINT_URL', 'https://s3.nl-ams.scw.cloud');

but with the second solution, we are able to easily configure both:

• endpoint URL for upload
• and the URL rewriting (if not explicitly configured)