openstack
/
manila
Code Issues Proposed changes

Remove unused attr status from models 

Models "security_services" and "network_allocations" have attr "status" that is
not used indeed. So, remove it from models and add appropriate migrations.

Closes-Bug: #1459660
Change-Id: Idb3a69916e8052b16c9daebb9bb67b09d1714c46
This commit is contained in:
Valeriy Ponomaryov 2015-05-28 13:38:05 +03:00
parent 51589d441c
commit eff9f92f01
7 changed files with 96 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
contrib/tempest/tempest/api/share/test_security_services.py
View File

@ -82,7 +82,6 @@ class SecurityServiceListMixin(object):
@test.attr(type=["gate", "smoke"])
def test_list_security_services_detailed_filter_by_ss_attributes(self):
search_opts = {
'status': 'NEW',
'name': 'ss_ldap',
'type': 'ldap',
'user': 'fake_user',

30
manila/api/v1/security_service.py
View File

@ -98,6 +98,9 @@ class SecurityServiceController(wsgi.Controller):
search_opts = {}
search_opts.update(req.GET)
# NOTE(vponomaryov): remove 'status' from search opts
# since it was removed from security service model.
search_opts.pop('status', None)
if 'share_network_id' in search_opts:
share_nw = db.share_network_get(context,
search_opts['share_network_id'])
@ -141,7 +144,7 @@ class SecurityServiceController(wsgi.Controller):
return security_services
def _get_security_services_search_options(self):
return ('status', 'name', 'id', 'type', 'user',
return ('name', 'id', 'type', 'user',
'server', 'dns_ip', 'domain', )
def _share_servers_dependent_on_sn_exist(self, context,
@ -173,23 +176,18 @@ class SecurityServiceController(wsgi.Controller):
except exception.NotFound:
raise exc.HTTPNotFound()
if security_service['status'].lower() in ['new', 'inactive']:
update_dict = security_service_data
if self._share_servers_dependent_on_sn_exist(context, id):
for item in update_dict:
if item not in valid_update_keys:
msg = _("Cannot update security service %s. It is "
"attached to share network with share server "
"associated. Only 'name' and 'description' "
"fields are available for update.") % id
raise exc.HTTPForbidden(explanation=msg)
else:
update_dict = dict([(key, security_service_data[key])
for key in valid_update_keys
if key in security_service_data])
if self._share_servers_dependent_on_sn_exist(context, id):
for item in security_service_data:
if item not in valid_update_keys:
msg = _("Cannot update security service %s. It is "
"attached to share network with share server "
"associated. Only 'name' and 'description' "
"fields are available for update.") % id
raise exc.HTTPForbidden(explanation=msg)
policy.check_policy(context, RESOURCE_NAME, 'update', security_service)
security_service = db.security_service_update(context, id, update_dict)
security_service = db.security_service_update(
context, id, security_service_data)
return self._view_builder.detail(req, security_service)
def create(self, req, body):

31
manila/api/views/security_service.py
View File

@ -14,6 +14,7 @@
# under the License.
from manila.api import common
from manila.common import constants
class ViewBuilder(common.ViewBuilder):
@ -36,29 +37,23 @@ class ViewBuilder(common.ViewBuilder):
'id': security_service.get('id'),
'name': security_service.get('name'),
'type': security_service.get('type'),
'status': security_service.get('status')
# NOTE(vponomaryov): attr "status" was removed from model and
# is left in view for compatibility purposes since it affects
# user-facing API. This should be removed right after no one
# uses it anymore.
'status': constants.STATUS_NEW,
}
}
def detail(self, request, security_service):
"""Detailed view of a single security service."""
return {
'security_service': {
'id': security_service.get('id'),
'name': security_service.get('name'),
'created_at': security_service.get('created_at'),
'updated_at': security_service.get('updated_at'),
'status': security_service.get('status'),
'description': security_service.get('description'),
'dns_ip': security_service.get('dns_ip'),
'server': security_service.get('server'),
'domain': security_service.get('domain'),
'user': security_service.get('user'),
'password': security_service.get('password'),
'type': security_service.get('type'),
'project_id': security_service.get('project_id'),
}
}
view = self.summary(request, security_service)
keys = (
'created_at', 'updated_at', 'description', 'dns_ip', 'server',
'domain', 'user', 'password', 'project_id')
for key in keys:
view['security_service'][key] = security_service.get(key)
return view
def _list_view(self, func, request, security_services):
"""Provide a view for a list of security services."""

68
manila/db/migrations/alembic/versions/533646c7af38_remove_unused_attr_status.py Normal file
View File

@ -0,0 +1,68 @@
# Copyright 2015 Mirantis, Inc.
#
# 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.
"""Remove unused attr status
Revision ID: 533646c7af38
Revises: 3a482171410f
Create Date: 2015-05-28 13:13:47.651353
"""
# revision identifiers, used by Alembic.
revision = '533646c7af38'
down_revision = '3a482171410f'
from alembic import op
from oslo_log import log
import sqlalchemy as sql
from manila.common import constants
from manila.i18n import _LE
LOG = log.getLogger(__name__)
COLUMN_NAME = 'status'
TABLE_NAMES = ('network_allocations', 'security_services')
def upgrade():
for t_name in TABLE_NAMES:
try:
op.drop_column(t_name, COLUMN_NAME)
except Exception:
LOG.error(_LE("Column '%s' could not be dropped"), COLUMN_NAME)
raise
def downgrade():
for t_name in TABLE_NAMES:
try:
op.add_column(
t_name,
sql.Column(
COLUMN_NAME,
# NOTE(vponomaryov): original type of attr was enum. But
# alembic is buggy with enums [1], so use string type
# instead. Anyway we have no reason to keep enum/constraint
# on specific set of possible statuses because they have
# not been used.
# [1] - https://bitbucket.org/zzzeek/alembic/
# issue/89/opadd_column-and-opdrop_column-should
sql.String(255),
default=constants.STATUS_NEW,
),
)
except Exception:
LOG.error(_LE("Column '%s' could not be added"), COLUMN_NAME)
raise

6
manila/db/sqlalchemy/models.py
View File

@ -349,9 +349,6 @@ class SecurityService(BASE, ManilaBase):
password = Column(String(255), nullable=True)
name = Column(String(255), nullable=True)
description = Column(String(255), nullable=True)
status = Column(Enum(constants.STATUS_NEW, constants.STATUS_ACTIVE,
constants.STATUS_ERROR),
default=constants.STATUS_NEW)
class ShareNetwork(BASE, ManilaBase):
@ -468,9 +465,6 @@ class NetworkAllocation(BASE, ManilaBase):
mac_address = Column(String(32), nullable=True)
share_server_id = Column(String(36), ForeignKey('share_servers.id'),
nullable=False)
status = Column(Enum(constants.STATUS_NEW, constants.STATUS_ACTIVE,
constants.STATUS_ERROR),
default=constants.STATUS_NEW)
class DriverPrivateData(BASE, ManilaBase):

3
manila/tests/api/v1/test_security_service.py
View File

@ -43,7 +43,7 @@ class ShareApiTest(test.TestCase):
"domain": "fake-domain",
"user": "fake-user",
"password": "fake-password",
"status": "new",
"status": constants.STATUS_NEW,
"project_id": "fake",
}
self.ss_ldap = {
@ -66,7 +66,6 @@ class ShareApiTest(test.TestCase):
'server': 'fake-server',
'dns_ip': '1.1.1.1',
'domain': 'fake-domain',
'status': 'new',
'type': constants.SECURITY_SERVICES_ALLOWED_TYPES[0],
}
self.check_policy_patcher = mock.patch(

3
manila/tests/network/test_security_service_db.py
View File

@ -15,7 +15,6 @@
from oslo_db import exception as db_exception
from manila.common import constants
from manila import context
from manila.db import api as db_api
from manila import exception
@ -33,7 +32,6 @@ security_service_dict = {
'password': 'fake password',
'name': 'whatever',
'description': 'nevermind',
'status': constants.STATUS_NEW,
}
@ -101,7 +99,6 @@ class SecurityServiceDBTest(test.TestCase):
'password': 'new password',
'name': 'new whatever',
'description': 'new nevermind',
'status': constants.STATUS_ERROR,
}
db_api.security_service_create(self.fake_context,