-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Integrate S3Interface with replicator #72
Conversation
Committer: Sarah Wooders <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just need some minor changes, but merge it after that! I fixed one or two minor typos which makes it work on my local machine.
@@ -223,9 +226,22 @@ def start_gateway( | |||
docker_out, docker_err = self.run_command(f"sudo docker pull {gateway_docker_image}") | |||
assert "Status: Downloaded newer image" in docker_out or "Status: Image is up to date" in docker_out, (docker_out, docker_err) | |||
|
|||
# read AWS config file to get credentials | |||
# TODO: Integrate this with updated skylark config file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One way to easily integrate configs would be to pass all of the config variables here from config.json. Feel free to open an issue linking this area of the code so we don't block this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made an issue #85
credential_provider = AwsCredentialsProvider.new_default_chain(bootstrap) | ||
|
||
# get aws auth info for docker envs | ||
aws_access_key_id = os.getenv("AWS_ACCESS_KEY_ID", None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use something like this to add a way to load credentials from the config.json as well:
from skylark.cli.cli_helper import load_config
config = load_config()
return config.get("aws_access_key_id"), config.get("aws_secret_access_key")
It seems to be working now (see my last commit for some minor changes):
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just need some minor changes, but merge it after that! I fixed one or two minor typos which makes it work on my local machine.
I've started getting an authentication error with the VPCs, which I'm guessing might be happening from too many boto3 requests (since the commands work fine in the CLI). I stopped the errors by commenting out all but the US regions, but that's probably not the fix we want...
|
* Reuse S3Interface, batch S3 delete calls to fix AWS bug * When getting object sizes, throttle requests to avoid retry errors * Types in Chunk should be Optional Co-authored-by: Sarah Wooders <[email protected]>
Are we good to merge this PR? I think we can just make the config stuff another GitHub issue and table it for now. |
This reverts commit f3cf3df.
src_object_store
anddst_object_store
as additional options for chunk location.S3Interface
for object store hopsS3Interface
(using the originalAwsCredentialsProvider.new_default_chain
with credentials copied to the gateway didn't work for unknown reasons)Potential Issues
GatewaySender
andGatewayReceiver
n_chunks
(only tested with--n-chunks 100
) because ofawscrt
error