Skip to content
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

CLOUDSTACK: Implemented VPC network acls and tests #371

Closed
wants to merge 2 commits into from

Conversation

@boul
Copy link
Contributor

boul commented Oct 9, 2014

VPC Support was already added in a previous commit, this commit also introduces support for ACLList on VPC's. Including unit tests. Thanks @jeroendekorte

% (self.id, self.displaytext, self.name,
self.vpc_offering_id, self.zoneid, self.cidr,
self.driver.name))
return (('<CloudStackVPC: id=%s, name=%s, '

This comment has been minimized.

Copy link
@sebgoa

sebgoa Oct 12, 2014

Member

Why did you remove the zone and display text ?

return (('<CloudStackVPC: id=%s, displaytext=%s, name=%s, '
'vpc_offering_id=%s, zoneid=%s, cidr=%s, driver%s>')
% (self.id, self.displaytext, self.name,
self.vpc_offering_id, self.zoneid, self.cidr,

This comment has been minimized.

Copy link
@sebgoa

sebgoa Oct 20, 2014

Member

@boul @jeroendekorte Why do you change the order of the variables in the class ? does it matter ? also the driver value is really the name of the driver. I haven't checked the other classes but we could return the full driver object.

This comment has been minimized.

Copy link
@jeroendekorte

jeroendekorte Oct 20, 2014

Contributor

@Runseb I've checked the code for the driver comment and I always pass the full driver object as a parameter.

This comment has been minimized.

Copy link
@boul

boul Oct 23, 2014

Author Contributor

fixed ordering see latest commit

self.name = name
self.vpc_id = vpc_id
self.description = description

This comment has been minimized.

Copy link
@sebgoa

sebgoa Oct 20, 2014

Member

@boul @jeroendekorte so this class does not return anything ?

This comment has been minimized.

Copy link
@boul

boul Oct 20, 2014

Author Contributor

@jeroendekorte can you take a look at the comments from @Runseb. Cheers!.

This comment has been minimized.

Copy link
@jeroendekorte

jeroendekorte Oct 20, 2014

Contributor

@Runseb Correct it does not return anything, is this something we should do? I use this class only as a data structure. Please correct me if I'm wrong.

@boul boul force-pushed the boul:vpcnetworks branch from 3a7b975 to bee43b3 Oct 23, 2014
@boul
Copy link
Contributor Author

boul commented Oct 23, 2014

@Runseb I fixed the repr ordering as you suggested, to match the object order. I kept driver.name as this seems common in the project, see EC2NodeLocation for example. And i'm not sure what will break if I change this.

class EC2NodeLocation(NodeLocation):
def init(self, id, name, country, driver, availability_zone):
super(EC2NodeLocation, self).init(id, name, country, driver)
self.availability_zone = availability_zone

def __repr__(self):
    return (('<EC2NodeLocation: id=%s, name=%s, country=%s, '
             'availability_zone=%s driver=%s>')
            % (self.id, self.name, self.country,
               self.availability_zone, self.driver.name))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.