Skip to content
This repository has been archived by the owner on Jun 5, 2023. It is now read-only.

Api additions - Get Networks #677

Merged
merged 6 commits into from
Oct 2, 2017
Merged

Api additions - Get Networks #677

merged 6 commits into from
Oct 2, 2017

Conversation

therealsmartalec
Copy link
Contributor

@therealsmartalec therealsmartalec commented Oct 2, 2017

Thanks for opening a Pull Request!

Here's a handy checklist to ensure your PR goes smoothly.

  • I signed Google's Contributor License Agreement
  • My code conforms to Google's python style.
  • My PR at a minimum doesn't decrease unit-test coverage (if applicable).
  • My PR has been functionally tested.
  • All of the unit-tests still pass.
  • Running pylint --rcfile=pylintrc passes.

These guidelines and more can be found in our contributing guidelines.

Adding the ability to get a list of networks via the api and tests to
verify data.
Re made the fake responses to work with pagenation.
Changed get_networks from beta to v1, removed the subnetworks test.
Forgot to remove the fake data for the subnetworks before last push.
@codecov
Copy link

codecov bot commented Oct 2, 2017

Codecov Report

Merging #677 into dev will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             dev     #677      +/-   ##
=========================================
+ Coverage   83.7%   83.72%   +0.02%     
=========================================
  Files        165      165              
  Lines       8359     8370      +11     
=========================================
+ Hits        6997     7008      +11     
  Misses      1362     1362
Impacted Files Coverage Δ
google/cloud/security/common/gcp_api/compute.py 100% <100%> (ø) ⬆️

Copy link
Collaborator

@ahoying ahoying left a comment

Choose a reason for hiding this comment

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

Thanks!

"kind": "compute#network",
"id": "1234",
"creationTimestamp": "2017-09-25T12:41:09.416-07:00",
"name": "default2",
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: name should match selflink, change from default2 to thisisatest and update the expected names below.

@@ -194,6 +195,16 @@ def firewalls(self):
return self._firewalls

@property
def networks(self):
"""Returns a _ComputeNetworksRepository instance."""
# The beta api provides more complete gcp networks data.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Comment is out of date, no longer needed with the change back to v1.

@@ -117,6 +117,17 @@ def test_get_firewall_rules_errors(self, name, response, status,
with self.assertRaises(expected_exception):
list(self.gce_api_client.get_firewall_rules(self.project_id))

def test_get_networks(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a test for the error cases as well, like test_get_firewall_rules_errors above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do!

Added the test_get_network_errors test to verify error condidtions for
get networks
@therealsmartalec therealsmartalec merged commit 824e1f9 into dev Oct 2, 2017
@therealsmartalec therealsmartalec deleted the api-additions branch October 2, 2017 19:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants