From c61cf6c4e57fe7526c4df01ec32eaeb06023dc21 Mon Sep 17 00:00:00 2001 From: Akim Juillerat Date: Wed, 27 Feb 2019 16:21:43 +0100 Subject: [PATCH 1/3] attachment_s3: Migrate to boto3 --- attachment_s3/__manifest__.py | 2 +- attachment_s3/models/ir_attachment.py | 123 ++++++++++++++------------ requirements.txt | 2 +- 3 files changed, 69 insertions(+), 58 deletions(-) diff --git a/attachment_s3/__manifest__.py b/attachment_s3/__manifest__.py index 7168700..dec7f77 100644 --- a/attachment_s3/__manifest__.py +++ b/attachment_s3/__manifest__.py @@ -10,7 +10,7 @@ 'category': 'Knowledge Management', 'depends': ['base', 'base_attachment_object_storage'], 'external_dependencies': { - 'python': ['boto'], + 'python': ['boto3'], }, 'website': 'https://www.camptocamp.com', 'data': [], diff --git a/attachment_s3/models/ir_attachment.py b/attachment_s3/models/ir_attachment.py index 513d07e..17124d4 100644 --- a/attachment_s3/models/ir_attachment.py +++ b/attachment_s3/models/ir_attachment.py @@ -5,7 +5,7 @@ import base64 import logging import os -import xml.dom.minidom +import io from odoo import _, api, exceptions, models from ..s3uri import S3Uri @@ -13,12 +13,13 @@ from ..s3uri import S3Uri _logger = logging.getLogger(__name__) try: - import boto - from boto.exception import S3ResponseError + import boto3 + from botocore.exceptions import ClientError, EndpointConnectionError except ImportError: - boto = None # noqa - S3ResponseError = None # noqa - _logger.debug("Cannot 'import boto'.") + boto3 = None # noqa + ClientError = None # noqa + EndpointConnectionError = None # noqa + _logger.debug("Cannot 'import boto3'.") class IrAttachment(models.Model): @@ -55,13 +56,9 @@ class IrAttachment(models.Model): 'aws_secret_access_key': secret_key, } if host: - params['host'] = host + params['endpoint_url'] = host if region_name: - # needs specific method for region - connect_s3 = boto.s3.connect_to_region params['region_name'] = region_name - else: - connect_s3 = boto.connect_s3 if not (access_key and secret_key and bucket_name): msg = _('If you want to read from the %s S3 bucket, the following ' 'environment variables must be set:\n' @@ -75,30 +72,34 @@ class IrAttachment(models.Model): ) % (bucket_name, bucket_name) raise exceptions.UserError(msg) - + # try: + s3 = boto3.resource('s3', **params) + bucket = s3.Bucket(bucket_name) + exists = True try: - conn = connect_s3(**params) - - except S3ResponseError as error: + s3.meta.client.head_bucket(Bucket=bucket_name) + except ClientError as e: + # If a client error is thrown, then check that it was a 404 error. + # If it was a 404 error, then the bucket does not exist. + error_code = e.response['Error']['Code'] + if error_code == '404': + exists = False + except EndpointConnectionError as error: # log verbose error from s3, return short message for user _logger.exception('Error during connection on S3') - raise exceptions.UserError(self._parse_s3_error(error)) + raise exceptions.UserError(str(error)) - bucket = conn.lookup(bucket_name) - if not bucket: - bucket = conn.create_bucket(bucket_name) + if not exists: + if not region_name: + bucket = s3.create_bucket(Bucket=bucket_name) + else: + bucket = s3.create_bucket( + Bucket=bucket_name, + CreateBucketConfiguration={ + 'LocationConstraint': region_name + }) return bucket - @staticmethod - def _parse_s3_error(s3error): - msg = s3error.reason - # S3 error message is a XML message... - doc = xml.dom.minidom.parseString(s3error.body) - msg_node = doc.getElementsByTagName('Message') - if msg_node: - msg = '%s: %s' % (msg, msg_node[0].childNodes[0].data) - return msg - @api.model def _store_file_read(self, fname, bin_size=False): if fname.startswith('s3://'): @@ -110,10 +111,16 @@ class IrAttachment(models.Model): "error reading attachment '%s' from object storage", fname ) return '' - filekey = bucket.get_key(s3uri.item()) - if filekey: - read = base64.b64encode(filekey.get_contents_as_string()) - else: + try: + key = s3uri.item() + bucket.meta.client.head_object( + Bucket=bucket.name, Key=key + ) + res = io.BytesIO() + bucket.download_fileobj(key, res) + res.seek(0) + read = base64.b64encode(res.read()) + except ClientError: read = '' _logger.info( "attachment '%s' missing on object storage", fname @@ -126,19 +133,21 @@ class IrAttachment(models.Model): def _store_file_write(self, key, bin_data): if self._storage() == 's3': bucket = self._get_s3_bucket() - filekey = bucket.get_key(key) or bucket.new_key(key) + obj = bucket.Object(key=key) + file = io.BytesIO() + file.write(bin_data) + file.seek(0) filename = 's3://%s/%s' % (bucket.name, key) try: - filekey.set_contents_from_string(bin_data) - except S3ResponseError as error: + obj.upload_fileobj(file) + except ClientError as error: # log verbose error from s3, return short message for user - _logger.exception( - 'Error during storage of the file %s' % filename - ) - raise exceptions.UserError( - _('The file could not be stored: %s') % - (self._parse_s3_error(error),) - ) + _logger.exception( + 'Error during storage of the file %s' % filename + ) + raise exceptions.UserError( + _('The file could not be stored: %s') % str(error) + ) else: _super = super(IrAttachment, self) filename = _super._store_file_write(key, bin_data) @@ -154,18 +163,20 @@ class IrAttachment(models.Model): # otherwise, we might delete files used on a different environment if bucket_name == os.environ.get('AWS_BUCKETNAME'): bucket = self._get_s3_bucket() - filekey = bucket.get_key(item_name) - if filekey: - try: - filekey.delete() - _logger.info( - 'file %s deleted on the object storage' % (fname,) - ) - except S3ResponseError: - # log verbose error from s3, return short message for - # user - _logger.exception( - 'Error during deletion of the file %s' % fname - ) + obj = bucket.Object(key=item_name) + try: + bucket.meta.client.head_object( + Bucket=bucket.name, Key=item_name + ) + obj.delete() + _logger.info( + 'file %s deleted on the object storage' % (fname,) + ) + except ClientError: + # log verbose error from s3, return short message for + # user + _logger.exception( + 'Error during deletion of the file %s' % fname + ) else: super(IrAttachment, self)._store_file_delete(fname) diff --git a/requirements.txt b/requirements.txt index e3c77a7..286d637 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -boto==2.42.0 +boto3==1.9.102 redis==2.10.5 python-json-logger==0.1.5 statsd==3.2.1 From 498aae2f5325531ca1ae442a3f9066208b80d4d9 Mon Sep 17 00:00:00 2001 From: Akim Juillerat Date: Tue, 26 Feb 2019 23:43:09 +0100 Subject: [PATCH 2/3] [IMP]: Allow to pass storage as a context key --- attachment_s3/models/ir_attachment.py | 3 ++- .../models/ir_attachment.py | 14 ++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/attachment_s3/models/ir_attachment.py b/attachment_s3/models/ir_attachment.py index 17124d4..9041989 100644 --- a/attachment_s3/models/ir_attachment.py +++ b/attachment_s3/models/ir_attachment.py @@ -131,7 +131,8 @@ class IrAttachment(models.Model): @api.model def _store_file_write(self, key, bin_data): - if self._storage() == 's3': + location = self.env.context.get('storage_location') or self._storage() + if location == 's3': bucket = self._get_s3_bucket() obj = bucket.Object(key=key) file = io.BytesIO() diff --git a/base_attachment_object_storage/models/ir_attachment.py b/base_attachment_object_storage/models/ir_attachment.py index deda46c..51303bd 100644 --- a/base_attachment_object_storage/models/ir_attachment.py +++ b/base_attachment_object_storage/models/ir_attachment.py @@ -42,8 +42,9 @@ class IrAttachment(models.Model): @api.cr def _register_hook(self): super(IrAttachment, self)._register_hook() + location = self.env.context.get('storage_location') or self._storage() # ignore if we are not using an object storage - if self._storage() not in self._get_stores(): + if location not in self._get_stores(): return curframe = inspect.currentframe() calframe = inspect.getouterframes(curframe, 2) @@ -102,7 +103,7 @@ class IrAttachment(models.Model): def _inverse_datas(self): # override in order to store files that need fast access, # we keep them in the database instead of the object storage - location = self._storage() + location = self.env.context.get('storage_location') or self._storage() for attach in self: if location in self._get_stores() and attach._save_in_db_anyway(): # compute the fields that depend on datas @@ -152,7 +153,8 @@ class IrAttachment(models.Model): @api.model def _file_write(self, value, checksum): - if self._storage() in self._get_stores(): + location = self.env.context.get('storage_location') or self._storage() + if location in self._get_stores(): bin_data = base64.b64decode(value) key = self._compute_checksum(bin_data) filename = self._store_file_write(key, bin_data) @@ -233,15 +235,15 @@ class IrAttachment(models.Model): if not self.env['res.users'].browse(self.env.uid)._is_admin(): raise exceptions.AccessError( _('Only administrators can execute this action.')) - storage = self._storage() - if storage not in self._get_stores(): + location = self.env.context.get('storage_location') or self._storage() + if location not in self._get_stores(): return super(IrAttachment, self).force_storage() self._force_storage_to_object_storage() @api.model def _force_storage_to_object_storage(self, new_cr=False): _logger.info('migrating files to the object storage') - storage = self._storage() + storage = self.env.context.get('storage_location') or self._storage() # The weird "res_field = False OR res_field != False" domain # is required! It's because of an override of _search in ir.attachment # which adds ('res_field', '=', False) when the domain does not From 626c1579646c177a5dd7bd263f8cf56231a2e8f9 Mon Sep 17 00:00:00 2001 From: Akim Juillerat Date: Fri, 1 Mar 2019 18:56:54 +0100 Subject: [PATCH 3/3] [IMP]: Allow to use context Key as storage key --- base_attachment_object_storage/models/ir_attachment.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/base_attachment_object_storage/models/ir_attachment.py b/base_attachment_object_storage/models/ir_attachment.py index 51303bd..85296c8 100644 --- a/base_attachment_object_storage/models/ir_attachment.py +++ b/base_attachment_object_storage/models/ir_attachment.py @@ -156,7 +156,9 @@ class IrAttachment(models.Model): location = self.env.context.get('storage_location') or self._storage() if location in self._get_stores(): bin_data = base64.b64decode(value) - key = self._compute_checksum(bin_data) + key = self.env.context.get('force_storage_key') + if not key: + key = self._compute_checksum(bin_data) filename = self._store_file_write(key, bin_data) else: filename = super(IrAttachment, self)._file_write(value, checksum)