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

fix(Enterprise Management): update service after recent API changes and add examples #102

Merged
merged 12 commits into from
Apr 13, 2021

Conversation

pyrooka
Copy link
Member

@pyrooka pyrooka commented Apr 8, 2021

PR summary

This PR updates the Enterprise Management service after recent API changes. Also adds working examples.

PR Checklist

Please make sure that your PR fulfills the following requirements:

  • The commit message follows the Angular Commit Message Guidelines.
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Current vs new behavior

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@codecov
Copy link

codecov bot commented Apr 8, 2021

Codecov Report

Merging #102 (40e6c76) into main (a1c5fa3) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #102   +/-   ##
=======================================
  Coverage   93.65%   93.65%           
=======================================
  Files          22       22           
  Lines       20641    20641           
=======================================
  Hits        19332    19332           
  Misses       1309     1309           
Impacted Files Coverage Δ
ibm_platform_services/enterprise_management_v1.py 95.78% <ø> (ø)
ibm_platform_services/version.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 938a74e...40e6c76. Read the comment docs.

@pyrooka pyrooka changed the title fix(Enterprise Management): update service after recent API changes fix(Enterprise Management): update service after recent API changes and add examples Apr 9, 2021
@pyrooka pyrooka requested a review from padamstx April 9, 2021 13:35

create_account_group_response = enterprise_management_service.create_account_group(
parent=parent,
name=first_account_group_name,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name=first_account_group_name,
name='Example Account Group',

global first_account_group_id
first_account_group_id = create_account_group_response.get('account_group_id')

create_second_account_group_response = enterprise_management_service.create_account_group(
Copy link
Member

Choose a reason for hiding this comment

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

formatting issues?

enterprise_account_iam_id = None

account_id = None
first_account_group_id = None
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
first_account_group_id = None
account_group_id = None


account_id = None
first_account_group_id = None
second_account_group_id = None
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
second_account_group_id = None
new_parent_account_group_id = None

# end-create_account_group

global first_account_group_id
first_account_group_id = create_account_group_response.get('account_group_id')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
first_account_group_id = create_account_group_response.get('account_group_id')
account_group_id = create_account_group_response.get('account_group_id')


response = enterprise_management_service.update_enterprise(
enterprise_id=enterprise_id,
name=updated_enterprise_name,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name=updated_enterprise_name,
name='Updated Example Enterprise',

list_account_groups_response = list_account_groups_response.get_result()
assert list_account_groups_response is not None

results_count += 1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
results_count += 1
results_count += list_account_groups_response.get('rows_count')

Yes, on the surface, this is equivalent to just incrementing the count by 1, but if we ever change limit, then you're innoculated :)

example_account_id = create_account_response.get('account_id')

@needscredentials
def test_list_accounts(self):
Copy link
Member

Choose a reason for hiding this comment

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

for list operations, I think testing the operation with pagination is sufficient since the "without pagination" scenario is a subset of the "with pagination" scenario.

assert update_account_response.get_status_code() == 202

@needscredentials
def test_list_enterprises(self):
Copy link
Member

Choose a reason for hiding this comment

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

Need to add pagination to this test

second_example_account_group_id = create_second_example_account_group_response.get('account_group_id')

@needscredentials
def test_list_account_groups(self):
Copy link
Member

Choose a reason for hiding this comment

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

we can remove this "without pagination" test

@CLAassistant
Copy link

CLAassistant commented Apr 12, 2021

CLA assistant check
All committers have signed the CLA.


response = enterprise_management_service.update_enterprise(
enterprise_id=enterprise_id,
name='Updated Enterprise Name',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name='Updated Enterprise Name',
name='Updated Example Enterprise',

@padamstx padamstx merged commit 6eaa326 into main Apr 13, 2021
@padamstx padamstx deleted the update-em branch April 13, 2021 19:07
ibm-devx-sdk pushed a commit that referenced this pull request Apr 13, 2021
## [0.18.5](v0.18.4...v0.18.5) (2021-04-13)

### Bug Fixes

* **Enterprise Management:** update service after recent API changes and add examples ([#102](#102)) ([6eaa326](6eaa326))
@ibm-devx-sdk
Copy link

🎉 This PR is included in version 0.18.5 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants