feat: Add S3 support for artifacts #556
Conversation
…torage client for improved testability; update tests to cover S3
samueljohnsiby
changed the title
samueljohnsiby
changed the title
I didn't see your PR, I opened a discussion about that. Are you done with it? Why it isn't merged yet? |
| def __init__(self, bucket_name: str,s3_client: Optional[BaseClient], **kwargs): | ||
| """Initializes the S3ArtifactService. | ||
|
|
||
| Args: | ||
| bucket_name: The name of the S3 bucket to use. | ||
| **kwargs: Optional parameters for boto3 client configuration. | ||
| """ | ||
| self.bucket_name = bucket_name | ||
| if s3_client is None: | ||
| self.s3 = boto3.client("s3", **kwargs) | ||
| else: | ||
| self.s3 = s3_client |
There was a problem hiding this comment.
Why are we giving the choice to not pass an S3 client directly but use **kwargs ? I think enforcing it is better.
|
Hi @samueljohnsiby thanks for implementing this PR. I am very interested in S3 compatibility. Do you plan to continue working on this? Given that you have already gotten the vast majority of functionality in here, I volunteer to take this forward for the last mile if you are otherwise occupied. |
|
Hi @samueljohnsiby and thanks to everyone who participated in this thread. We're currently doing some housekeeping on our pull request queue. Given the rapid pace of development and the number of updates since this was last active, there's a good chance this pull request needs to be caught up to the latest version. To help us keep our backlog focused on current pull requests, we are closing this as stale. If you're still interested in contributing to this change, could you please update to the very latest version of the library and create a new pull request? Our team will be glad to help with the refreshed PRs and answer any questions! Thanks for your contribution and understanding! |
5 participants
Added s3 support to manage artifacts using boto3. Written tests for the same under test artifact service file.
resolves : #555