Skip to content
This repository was archived by the owner on Dec 7, 2022. It is now read-only.

Support putting a publicly readable file to S3 bucket - #166

Open
njsubedi wants to merge 1 commit into
Slynova-Org:developfrom
njsubedi:patch-1
Open

Support putting a publicly readable file to S3 bucket#166
njsubedi wants to merge 1 commit into
Slynova-Org:developfrom
njsubedi:patch-1

Conversation

@njsubedi

Copy link
Copy Markdown

Add an optional boolean parameter named makePublic to the put() method. If makePublic is public, sets ACL: 'public-read' on the params, otherwise sets it to 'private', which is the default behavior. This change is backward compatible.

This PR does not break anything.

Add an optional boolean parameter named `makePublic` to the `put()` method. If `makePublic` is public, sets `ACL: 'public-read'` on the params, otherwise sets it to 'private', which is the default behavior. This change is backward compatible.
@njsubedi njsubedi changed the title Support publicly readable file Support putting a publicly readable file to S3 bucket Jul 20, 2021
@axmad386

axmad386 commented Jan 24, 2022

Copy link
Copy Markdown

Need this PR to be merged 🥲

@wilforlan

Copy link
Copy Markdown

Is anyone merging this PR soon?

@axmad386

axmad386 commented Jan 30, 2022

Copy link
Copy Markdown

Is anyone merging this PR soon?

I made fork of this package @kodepandai/flydrive. You can use it with @kodepandai/flydrive-s3 driver.
In put method, there are additional s3 PutObjectRequest on 3rd params

storage.put(path, content, {
   ContentType: 'image/jpg',
   ACL: 'public-read', //this for make file public
   CacheControl: ....,
   ...etc
})

But, my package only works on Esmodule. I made it for my internal project

@jthomaschewski

Copy link
Copy Markdown
storage.put(path, content, {
   ContentType: 'image/jpg',
   ACL: 'public-read', //this for make file public
   CacheControl: ....,
   ...etc
})

I'd personally prefer having this third option object with various options rather than making the 3rd parameter a boolean only for the purpose of making the file public.

Thanks for the fork @axmad386 - do you plan to do a PR to have this merged in the original project?

@axmad386

axmad386 commented Mar 19, 2022

Copy link
Copy Markdown
storage.put(path, content, {
   ContentType: 'image/jpg',
   ACL: 'public-read', //this for make file public
   CacheControl: ....,
   ...etc
})

I'd personally prefer having this third option object with various options rather than making the 3rd parameter a boolean only for the purpose of making the file public.

Thanks for the fork @axmad386 - do you plan to do a PR to have this merged in the original project?

@jthomaschewski I am not sure. I think my fork will break the original one. It's because I am focusing in esm mode. Except I must refactor my code maybe using rollup to support both esm and cjs. But I don't have much time for now. Sorry

@sagarPakhrin

Copy link
Copy Markdown

Any update on this?
I'm uploading profile pictures on gcs and I need the pictures to be public
If not merged, what's a workaround to make a file public?

@njsubedi

njsubedi commented Jul 6, 2022

Copy link
Copy Markdown
Author

You can create a custom class that extends this class and override the put function.

export class YourAWSStorage extends AmazonWebServicesS3Storage {
        /**
	 * Creates a new file.
	 * This method will create missing directories on the fly.
	 */
	public async put(location: string, content: Buffer | NodeJS.ReadableStream | string, makePublic: boolean = false): Promise<Response> {
		const params = { Key: location, Body: content, Bucket: this.$bucket, ACL: makePublic ? 'public-read' : 'private' };
		try {
			const result = await this.$driver.upload(params).promise();
			return { raw: result };
		} catch (e) {
			throw handleError(e, location, this.$bucket);
		}
	}

}

@wilforlan

Copy link
Copy Markdown

Great idea!! @njsubedi, how did I not think about that haha? Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants