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

DimesionData: Generic pagination, anti-affinity rules, create firewall expansion #726

Closed
wants to merge 6 commits into from

Conversation

@jadunham1
Copy link
Contributor

@jadunham1 jadunham1 commented Mar 24, 2016

Added:
ex_list_anti_affinity_rules
ex_create_anti_affinity_rule
ex_delete_anti_affinity_rule
generic pagination function
modified ex_create_firewall_rule to be able to place rules in a specific position relative to another rule

Tests for all new added features
Added some more coverage tests to ex_create_firewall rule
97% coverage in our compute driver now.


rules = []
for result in paged_result:
rules.extend(self._to_anti_affinity_rules(result))

This comment has been minimized.

@tonybaloney

tonybaloney Mar 24, 2016
Contributor

would probably be quicker to use list comprehension here

This comment has been minimized.

@jadunham1

jadunham1 Mar 24, 2016
Author Contributor

I Think since our to methods are returning lists here themselves extend makes more sense than list comprehension. If you have a better way let me know because all of our functions will have the same type of style in them so we should get it right the first time.

@tonybaloney
Copy link
Contributor

@tonybaloney tonybaloney commented Mar 24, 2016

changes look good. once tests are supporting I will test and merge.

@jadunham1 jadunham1 changed the title [WIP] DimesionData: Generic pagination, anti-affinity rules, create firewall expansion DimesionData: Generic pagination, anti-affinity rules, create firewall expansion Mar 24, 2016
"""
def __init__(self, id, node_list):
self.id = id
self.node_list = node_list

This comment has been minimized.

@tonybaloney

tonybaloney Mar 25, 2016
Contributor

Can you docstring what this does/type etc please.

This comment has been minimized.

@jadunham1

jadunham1 Mar 25, 2016
Author Contributor

Whoops missed that one

@tonybaloney
Copy link
Contributor

@tonybaloney tonybaloney commented Mar 25, 2016

Excellent changes, great to see some improvement in the tests as well. We'll need this in a few weeks when the 2.2 API comes out. Only 1 small change about docstring then we're good to merge

@tonybaloney
Copy link
Contributor

@tonybaloney tonybaloney commented Mar 26, 2016

thanks LGTM

@asfgit asfgit closed this in 03733cc Mar 26, 2016
asfgit pushed a commit that referenced this pull request Mar 26, 2016
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

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