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 for query_dn method with "hierarchy=True" #1

Merged
merged 1 commit into from
Jun 1, 2016

Conversation

prvashis
Copy link
Contributor

  • Fix for "query_dn" method failing when parameter "hierarchy=True".
  • Added checks to validate that SDK allows connection only to IMC Servers.
  • PEP8 changes.

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.04% when pulling 1010e11 on prvashis:dn_fix into 500504c on CiscoUcs:master.

@vvb
Copy link
Member

vvb commented May 26, 2016

@prvashis can you investigate the 2 code smells above and update. Cyclic dependency specifically.

@vvb
Copy link
Member

vvb commented May 27, 2016

@prvashis I investigated the cyclic import issue - it is a false positive due to a bug in pylint - pylint-dev/pylint#850
My last commit will take care of it

if len(method_response.out_config.child) == 0:
return mo_list
else:
if len(method_response.out_configs.child) == 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

@prvashis : does imc support out_configs? if not please remove else block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

configResolveClass API returns "outConfigs" where as configResolveDn API returns "outConfig" , because of this we have put the above if else statement.

Copy link
Member

Choose a reason for hiding this comment

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

@prvashis from what I remember for ucsm, configResolveClass = > outconfig, configResolveClasses => outConfigs.. so, you mean IMC is different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vvb Yes in IMC we have "configResolveClass","configResolveChildren" and "configResolveDn".
We don't have configResolveClasses and configResolveDns in IMC.

configResolveDn returns "outConfig"

Whereas configResolveClass and configResolveChildren returns "outConfigs"

Copy link
Member

Choose a reason for hiding this comment

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

@prvashis I stand corrected.. I think configResolveClass in ucsm also returns outConfigs, as it could return multiple Mos, even if one class was mentioned.

PEP8 changes.

Added checks to validate that SDK allows connection only to IMC Servers.

Incorporated code review comments.

Incorporated comments on Dn fix issue.

Incorporated review comments.
@vvb
Copy link
Member

vvb commented Jun 1, 2016

@prvashis LGTM, thanks for the patience in resolving all the review comments.

@vvb vvb merged commit 62f8669 into CiscoUcs:master Jun 1, 2016
techyragu pushed a commit to techyragu/imcsdk that referenced this pull request Sep 19, 2017
…ens to master

* commit '8c1782bb958f8d40cba81aa2f383670c0d6e113c':
  unit tests for bios tokens api
  correcting the verbs for the MOs
  correcting the version for M5 schema to 311d
  Bios Token APIs and generated helper file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants