jeanbmar / s3-sync-client

AWS CLI s3 sync command for Node.js
MIT License
80 stars 22 forks source link

Filters option different from AWS CLI #30

Closed rktyt closed 1 year ago

rktyt commented 2 years ago
// aws s3 sync --dryrun --delete --exclude 'excl/*' /path/to/local/dir s3://my-target-bucket
// Note: The results of the above command and the following code are different
import { S3Client } from "@aws-sdk/client-s3";
import S3SyncClient from "s3-sync-client";

process.env.S3_BACKET_NAME = "my-target-bucket";
process.env.S3_REGION = "us-east-1";
const sourceDir = "/path/to/local/dir";

const client = new S3Client({ region: process.env.S3_REGION });
const { sync } = new S3SyncClient({ client });

const res = await sync(sourceDir, `s3://${process.env.S3_BACKET_NAME}`, {
  filters: [
    {
      exclude(key) {
        return key.startsWith("excl/");
      },
    },
  ],
  del: true,
  partSize: 100 * 1024 * 1024,
  dryRun: true,
});

console.log(res);

In the case of AWS CLI, excl/path/to/file that exists on the S3 side is not deleted, whereas it is deleted when using s3-sync-client. There is probably no problem with this specification, but please make it clear in the documentation.


Versions aws cli: 2.4.3 s3-sync-client: 3.0.0

jeanbmar commented 2 years ago

You are right and this behavior will cause issues for people doing a 1-1 transition.

I will think about a solution to provide options for both current and regular behaviors.

Ref for ideas: https://github.com/aws/aws-cli/issues/4923

Luay-Sol commented 1 year ago

@jeanbmar any update on the above? It will be great if this bug can be fixed, as IMO it is a necessary functionality. E.g I have adjusted the bucket-with-bucket.js code and added this code block, after finding the diff :

    deleted.forEach((deleted) => deleted.applyDeleteFilters(filters));
    const excludedDeletedObjects = sourceObjects.filter((sourceObject) => !sourceObject.isIncluded())

I think it covers most of the use cases, as in the case of the deletion, everything will be included by default, so we need to apply only the exclude to items that need to be deleted.

if such a solution make sense to you, I can open a PR and have it done properly.

jeanbmar commented 1 year ago

Fixed in 4.2.0, now matching CLI behavior by default. A new deleteExcluded option is also available.

https://github.com/jeanbmar/s3-sync-client/blob/b05c1d7bc18cd6fea96f823d92ad6fb4ccc15173/test/S3SyncClient.test.ts#L252-L285