Contrib.social

Has versioning been removed from the d8+ versions of the module

Open on Drupal.org →
Created on 6 April 2023, over 1 year ago
Updated 14 June 2024, 6 months ago

Problem/Motivation

The D7 version of the site had 2 settings s3fs_use_versioning which seemed to be replaced with use_versioning. These looked like they seemed to control the adding of the VersionId on assets.

I have scoured the code of the 8.x-3.x and can't see it. I also can't see any tickets that removed it. I haven't checked the 4.x, but suspect if it is missing from 3, 4 won't have it either.

I can see the settings being added in:
#3156257: Allow S3 Versioning as a Configurable Option →
#2958566: Allow S3 Versioning as a Configurable Option →

Steps to reproduce

Upload an asset to an S3 bucket that has versioning switched on and the assets, you can see that the VersionId gets added to the assets URL.

I would expect that if you set use_versioning to FALSE, then the assets should not be set to versioned in S3 or at least the VersionId should not appear on the URL.

Proposed resolution

Return the missing check:

    $meta = $this->_read_cache($this->uri);
    if (!empty($meta['version']) && !empty($this->config['use_versioning'])) {
      $external_url = $this->_append_get_arg($external_url, $meta['version']);
    }

Remaining tasks

Make a patch
Test and review the patch

User interface changes

None (Or maybe add a field in the settings form to allow the change)

API changes

None

Data model changes

None

💬 Support request
Status

Active

Version

3.0

Component

Code

Created by

🇬🇧United Kingdom the_g_bomb

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @the_g_bomb
  • Comment over 1 year ago →
    🇬🇧United Kingdom the_g_bomb
  • @the_g_bomb opened merge request.
  • Comment over 1 year ago →
    🇺🇸United States cmlara

    It isn't so much that it was removed from D8 as that it was brought into the code in a different manner, see #3163285: Don't require use of ListObjectVersions → .

    I recall questioning a time or two if we should be emitting the versionId in the URL if the version sync is disabled though never dug deep enough to see #3156257: Allow S3 Versioning as a Configurable Option → had done that after the 8.x-3.x branch had diverged from the 7.x-3.x branch.

    As currently exists there is no 'use_versioning' setting in 3.x and its nearest equivalent is named 'disable_version_sync' so this config parameter will not actually make any changes.

    Adding this in would be more of a feature request at this time and should probably target the 4.x branch first. That should also probably be discussed in context of should we actually be using more versioning data when its available (such as stream_open()) to ensure we don't end up with issues where if a file were updated without informing s3fs that could lead to either corrupt data or errors and is disabeling versioning actually better done outside s3fs at the bucket level.

  • Comment 6 months ago →
    🇬🇧United Kingdom Alina Basarabeanu

    We used the code changes from merge request 29 and worked as expected on Drupal core 10.2.2 and S3fs 3.4.0.
    We did not want to have the VersionId attached to the file

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024