Skip to content

Commit

Permalink
workaround for boto/boto3#125
Browse files Browse the repository at this point in the history
  • Loading branch information
mr-c committed Jul 17, 2021
1 parent 165afd0 commit 42f1b94
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 12 deletions.
1 change: 1 addition & 0 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ integration:
- python setup_gitlab_ssh.py
- chmod 400 /root/.ssh/id_rsa
- make test tests=src/toil/test/jobStores/jobStoreTest.py
- make test tests=src/toil/test/lib/aws/test_s3.py

provisioner_integration:
stage: integration
Expand Down
16 changes: 9 additions & 7 deletions src/toil/jobStores/aws/jobStore.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
from toil.jobStores.utils import (ReadablePipe,
ReadableTransformingPipe,
WritablePipe)
from toil.lib.aws.utils import create_s3_bucket
from toil.lib.compatibility import compat_bytes
from toil.lib.ec2 import establish_boto3_session
from toil.lib.ec2nodes import EC2Regions
Expand Down Expand Up @@ -744,11 +745,9 @@ def bucket_creation_pending(error):
bucketExisted = False
logger.debug("Bucket '%s' does not exist.", bucket_name)
if create:
logger.debug("Creating bucket '%s'.", bucket_name)
location = region_to_bucket_location(self.region)
bucket = self.s3_resource.create_bucket(
Bucket=bucket_name,
CreateBucketConfiguration={'LocationConstraint': location})
bucket = create_s3_bucket(
self.s3_resource, bucket_name, self.region
)
# Wait until the bucket exists before checking the region and adding tags
bucket.wait_until_exists()

Expand All @@ -757,7 +756,9 @@ def bucket_creation_pending(error):
# produce an S3ResponseError with code
# NoSuchBucket. We let that kick us back up to the
# main retry loop.
assert self.getBucketRegion(bucket_name) == self.region
assert (
self.getBucketRegion(bucket_name) == self.region
), f"bucket_name: {bucket_name}, {self.getBucketRegion(bucket_name)} != {self.region}"

owner_tag = os.environ.get('TOIL_OWNER_TAG')
if owner_tag:
Expand Down Expand Up @@ -1637,7 +1638,8 @@ def _delete_domain(self, domain):
if not no_such_sdb_domain(e):
raise

def _delete_bucket(self, bucket):
@staticmethod
def _delete_bucket(bucket):
"""
:param bucket: S3.Bucket
"""
Expand Down
2 changes: 1 addition & 1 deletion src/toil/jobStores/aws/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -441,4 +441,4 @@ def region_to_bucket_location(region):


def bucket_location_to_region(location):
return 'us-east-1' if location == '' else location
return "us-east-1" if location == "" or location is None else location
16 changes: 16 additions & 0 deletions src/toil/lib/aws/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@

try:
from boto.exception import BotoServerError
from boto3.resources.factory import s3
except ImportError:
BotoServerError = None # AWS/boto extra is not installed
s3 = None

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -78,3 +80,17 @@ def delete_s3_bucket(bucket: str, region: Optional[str], quiet: bool = True):
s3_client.delete_object(Bucket=bucket, Key=version['Key'], VersionId=version['VersionId'])
s3_resource.Bucket(bucket).delete()
printq(f'\n * Deleted s3 bucket successfully: {bucket}\n\n', quiet)


def create_s3_bucket(
s3_session: "s3.ServiceResource", bucket_name: str, region: str
) -> "s3.Bucket":
logger.debug("Creating bucket '%s' in region %s.", bucket_name, region)
if region == "us-east-1": # see https://github.com/boto/boto3/issues/125
bucket = s3_session.create_bucket(Bucket=bucket_name)
else:
bucket = s3_session.create_bucket(
Bucket=bucket_name,
CreateBucketConfiguration={"LocationConstraint": region},
)
return bucket
12 changes: 8 additions & 4 deletions src/toil/test/jobStores/jobStoreTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
NoSuchJobException)
from toil.jobStores.fileJobStore import FileJobStore
from toil.lib.memoize import memoize
from toil.lib.aws.utils import create_s3_bucket
from toil.statsAndLogging import StatsAndLogging
from toil.test import (ToilTest,
make_tests,
Expand Down Expand Up @@ -1408,14 +1409,17 @@ def _hashTestFile(self, url: str) -> str:
def _createExternalStore(self):
"""A S3.Bucket instance is returned"""
from toil.jobStores.aws.utils import retry_s3
from toil.jobStores.aws.utils import region_to_bucket_location
from toil.jobStores.aws.jobStore import establish_boto3_session
resource = establish_boto3_session().resource('s3', region_name=self.awsRegion())
bucket = resource.Bucket('import-export-test-%s' % uuid.uuid4())

resource = establish_boto3_session().resource(
"s3", region_name=self.awsRegion()
)
bucket_name = f"import-export-test-{uuid.uuid4()}"
location = self.awsRegion()

for attempt in retry_s3():
with attempt:
bucket.create(CreateBucketConfiguration={'LocationConstraint': region_to_bucket_location(self.awsRegion())})
bucket = create_s3_bucket(resource, bucket_name, location)
bucket.wait_until_exists()
return bucket

Expand Down
Empty file.
53 changes: 53 additions & 0 deletions src/toil/test/lib/aws/test_s3.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Copyright (C) 2021 Michael R. Crusoe
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
import os

from toil.jobStores.aws.jobStore import AWSJobStore
from toil.lib.aws.utils import create_s3_bucket
from toil.lib.ec2 import establish_boto3_session
from toil.test import ToilTest, needs_aws_s3

logger = logging.getLogger(__name__)
logging.basicConfig(level=logging.DEBUG)


@needs_aws_s3
class S3Test(ToilTest):
"""Confirm the workarounds for us-east-1."""

@classmethod
def setUpClass(cls):
session = establish_boto3_session(region_name="us-east-1")
cls.s3_resource = session.resource("s3", region_name="us-east-1")
cls.s3_client = cls.s3_resource.meta.client
cls.bucket = None

def test_create_bucket(self):
"""Test bucket creation for us-east-1."""
bucket_name = f"toil-S3Test-{uuid.uuid4()}"
cls.bucket = create_s3_bucket(cls.s3_resource, bucket_name, "us-east-1")
cls.bucket.wait_until_exists()
owner_tag = os.environ.get("TOIL_OWNER_TAG")
if owner_tag:
bucket_tagging = cls.s3_resource.BucketTagging(bucket_name)
bucket_tagging.put(
Tagging={"TagSet": [{"Key": "Owner", "Value": owner_tag}]}
)
self.assertTrue(AWSJobStore.getBucketRegion, "us-east-1")

@classmethod
def tearDownClass(cls):
AWSJobStore._delete_bucket(cls.bucket)
super(S3Test, cls).tearDownClass()

0 comments on commit 42f1b94

Please sign in to comment.