search

danielfrg / s3contents

Jupyter Notebooks in S3 - Jupyter Contents Manager implementation
Apache License 2.0
248 stars 88 forks source link

ssm = "aws:kms" - is there a way to specify SSEKMSKeyId for s3fs? #61

Closed nemcidan closed 5 years ago

nemcidan commented 5 years ago

I was not able to find whether s3contents supports "aws:kms" sse type. I was unable to find a way to specify the key ID for KMS.

in the jupyter configuration, i can specify sse type to be used:

from s3contents import S3ContentsManager

config = get_config()
...
config.S3ContentsManager.sse = "aws:kms"

which works, but files and folders are not readable. I suppose it is because KMS key ID is not specified.

After some digging in the source code, I found that this line configures the SSE. is there a possibility to add the funcionality for SSEKMSKeyId to be specified all the way from the jupyter config, something like this?

s3_additional_kwargs["SSEKMSKeyId"] = self.sse_kms_key_id

example config:

...
config.S3ContentsManager.sse = "aws:kms"
config.S3ContentsManager.sse_kms_key_id = "id"
...
danielfrg commented 5 years ago

That's not a valid option right now but it would be super useful to have it, it should also be relatively easy to add. As you already found the code that needs to be extended.

If you change the code and test it i'll be super happy to accept a Pull Request :)

hocanint-amzn commented 5 years ago

We needed this so we entered https://github.com/danielfrg/s3contents/pull/77. Thanks!

hocanint-amzn commented 5 years ago

@danielfrg Can we cut a new release and resolve this issue? Thanks

danielfrg commented 5 years ago

I just released 0.3.1 that includes the fix. https://pypi.org/project/s3contents/0.3.1/

Thanks for the PR!