Skip to content
This repository has been archived by the owner on Nov 28, 2017. It is now read-only.

Add a new instance action: Resize #29

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add a new instance action: Resize #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7f7ef76
add new instance action: resize
xuhang57 Mar 9, 2017
015140e
update on the backend
xuhang57 Mar 13, 2017
3d21f68
delete unused imported lib
xuhang57 Mar 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions api/v1/serializers/instance_serializer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class InstanceSerializer(serializers.ModelSerializer):
activity = serializers.CharField(read_only=True, source='esh_activity')
fault = serializers.ReadOnlyField(source='esh_fault')
size_alias = serializers.CharField(read_only=True, source='esh_size')
size = serializers.SerializerMethodField()
machine_alias = serializers.CharField(read_only=True, source='esh_source')
machine_name = serializers.CharField(read_only=True,
source='esh_source_name')
Expand Down Expand Up @@ -51,6 +52,14 @@ def __init__(self, *args, **kwargs):
self.request_user = user
super(InstanceSerializer, self).__init__(*args, **kwargs)

def get_size(self, obj):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find where this new function comes into play. Does it get called in some way by your frontend code?

Copy link
Collaborator Author

@xuhang57 xuhang57 Mar 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kamfonik Yes, so get_size gets called when we havesize = SerializerMethodField (Line 26) it will call def get_<field-name>

from api.v2.serializers.summaries import (
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be imported at the top?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kamfonik hmm, I will ask you on this then. Since this is only used by this function. Sometimes I see people import them within only the functions. But we could also put them on top. What do you think, Laura?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PEP8 recommends imports only at the top of the file, but I can see that there are already function-level imports elsewhere in this code.

From what I have read there may be a slight performance cost to importing inside the function, but probably only on the first call to the function (after that the module should be cached).

In discussions about this on StackOverflow there were a few notes that some people do import within the function while developing, until they have some stable code and know what they need and where it is used, then they move all the imports to the top to comply with PEP8.

Mostly my thought is that if we are going to violate PEP8 there should be a reason, although the reason could be simple like "consistency with existing code" etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kamfonik Thanks for researching on this. I could clean up the functions that I have modified/created. As for other elsewhere in code base. We either need to have a new PR for cleaning all the code, or keep ignoring them, but make sure we do it properly for the furture PRs.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xuhang57 sure, cleaning up is probably not urgent, but deciding now what you want for new code will save a lot of work later.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it should be moved to the top

SizeSummarySerializer
)
size = obj.get_size()
serializer = SizeSummarySerializer(size, context=self.context)
return serializer.data

class Meta:
model = Instance
exclude = ('source', 'provider_alias',
Expand Down
21 changes: 5 additions & 16 deletions service/instance.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,12 @@ def resize_instance(esh_driver, esh_instance, size_alias,
provider_uuid, identity_uuid, user):
_permission_to_act(identity_uuid, "Resize")
size = esh_driver.get_size(size_alias)
redeploy_task = resize_and_redeploy(
finish_resize_task = resize_and_redeploy(
esh_driver,
esh_instance,
identity_uuid)
esh_driver.resize_instance(esh_instance, size)
redeploy_task.apply_async()
finish_resize_task.apply_async()
# Write build state for new size
update_status(
esh_driver,
Expand Down Expand Up @@ -358,7 +358,6 @@ def resize_and_redeploy(esh_driver, esh_instance, core_identity_uuid):
Use this function to kick off the async task when you ONLY want to deploy
(No add fixed, No add floating)
"""
from service.tasks.driver import deploy_init_to
from service.tasks.driver import wait_for_instance, complete_resize
from service.deploy import deploy_test
touch_script = deploy_test()
Expand All @@ -367,21 +366,11 @@ def resize_and_redeploy(esh_driver, esh_instance, core_identity_uuid):
task_one = wait_for_instance.s(
esh_instance.id, esh_driver.__class__, esh_driver.provider,
esh_driver.identity, "verify_resize")
raise Exception("Programmer -- Fix this method based on the TODO")
# task_two = deploy_script.si(
# esh_driver.__class__, esh_driver.provider,
# esh_driver.identity, esh_instance.id, touch_script)
task_three = complete_resize.si(
task_two = complete_resize.si(
esh_driver.__class__, esh_driver.provider,
esh_driver.identity, esh_instance.id,
core_identity.provider.id, core_identity.id, core_identity.created_by)
task_four = deploy_init_to.si(
esh_driver.__class__, esh_driver.provider,
esh_driver.identity, esh_instance.id, core_identity, redeploy=True)
# Link em all together!
core_identity.provider.uuid, core_identity.uuid, core_identity.created_by)
task_one.link(task_two)
task_two.link(task_three)
task_three.link(task_four)
return task_one


Expand Down Expand Up @@ -1860,7 +1849,7 @@ def run_instance_action(user, identity, instance_id, action_type, action_params)
identity_uuid = identity.uuid
logger.info("User %s has initiated instance action %s to be executed on Instance %s" % (user, action_type, instance_id))
if 'resize' == action_type:
size_alias = action_params.get('size', '')
size_alias = action_params.get('resize_size', '')
if isinstance(size_alias, int):
size_alias = str(size_alias)
result_obj = resize_instance(
Expand Down