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

Fixes #21964 - Reduce calls to external API #6321

Merged

Conversation

Projects
None yet
6 participants
@justahero
Copy link
Contributor

justahero commented Dec 11, 2018

During a performance analysis of the host edit view rendering, a few redundant API calls were made in some cases. This PR minimizes the number of calls made to ComputeResource#find_vm_by_uuid, e.g. called by functions Orchestration::Compute#compute_object or ComputeResourcesVmsHelper#new_vm?.

The host edit template may send multiple redundant HTTP requests to an external API. Reducing the number of calls should improve overall performance.

  • use @vm variable when set in ComputeResourcesVmsController#import
  • reduce number of calls to ComputeResourcesVmsHelper#new_vm?
  • reduce number of calls to Orchestration::Compute#compute_object
  • re-use variable new_vm passed to partial
@theforeman-bot

This comment has been minimized.

Copy link
Member

theforeman-bot commented Dec 11, 2018

Do not merge! This patch has not been tested yet.

Can an existing organization member please verify this patch?

1 similar comment
@theforeman-bot

This comment has been minimized.

Copy link
Member

theforeman-bot commented Dec 11, 2018

Do not merge! This patch has not been tested yet.

Can an existing organization member please verify this patch?

@theforeman-bot

This comment has been minimized.

Copy link
Member

theforeman-bot commented Dec 11, 2018

There were the following issues with the commit message:

  • b59e998 must be in the format fixes #redmine_number - brief description
  • commit message for b59e998 is not wrapped at 72nd column
  • commit message for b59e998 is not wrapped at 72nd column
  • 75246e3 must be in the format fixes #redmine_number - brief description
  • commit message for 75246e3 is not wrapped at 72nd column
  • c8c38aa must be in the format fixes #redmine_number - brief description
  • f918c1f must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.


This message was auto-generated by Foreman's prprocessor

@theforeman-bot

This comment has been minimized.

Copy link
Member

theforeman-bot commented Dec 11, 2018

Do not merge! This patch has not been tested yet.

Can an existing organization member please verify this patch?

@justahero justahero changed the title Fixes 21964 - Reduce calls to external API Fixes #21964 - Reduce calls to external API Dec 11, 2018

@justahero justahero force-pushed the justahero:refactor-avoid-calling-vmware-api branch from f918c1f to ec1e07a Dec 11, 2018

@theforeman-bot

This comment has been minimized.

Copy link
Member

theforeman-bot commented Dec 11, 2018

There were the following issues with the commit message:

  • aec4aed must be in the format fixes #redmine_number - brief description
  • d1e7286 must be in the format fixes #redmine_number - brief description
  • c475f5a must be in the format fixes #redmine_number - brief description
  • ec1e07a must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.


This message was auto-generated by Foreman's prprocessor

@justahero justahero force-pushed the justahero:refactor-avoid-calling-vmware-api branch from ec1e07a to ae5275c Dec 11, 2018

@timogoebel
Copy link
Member

timogoebel left a comment

Makes sense.

@mmoll

This comment has been minimized.

Copy link
Member

mmoll commented Dec 11, 2018

ok to test

@ohadlevy

This comment has been minimized.

Copy link
Member

ohadlevy commented Dec 12, 2018

thanks @justahero this looks reasonable, can anyone confirm it works as expected in other compute resources?
@justahero can you please squash your commits? thanks.

Sebastian Ziebell
Fixes #21964 - extract calculation of new_vm to local variable
* prepare to pass the value if a vm is newly created
  or not to the template
* use variable `@vm` when present, avoids fetching host twice
* re-use status of `new_vm`, set once
* avoid calling `Host#compute_object` when possible parameter.

@justahero justahero force-pushed the justahero:refactor-avoid-calling-vmware-api branch from 50ca9db to 4967ca0 Dec 12, 2018

@ohadlevy ohadlevy merged commit 6c1f956 into theforeman:develop Dec 25, 2018

6 of 7 checks passed

upgrade Build finished. No test results found.
Details
Hound No violations found. Woof!
codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
foreman Build finished. 36720 tests run, 15 skipped, 0 failed.
Details
katello Build finished. 4116 tests run, 9 skipped, 0 failed.
Details
prprocessor Commit message style is correct
Details
@tbrisker

This comment has been minimized.

Copy link
Member

tbrisker commented Dec 25, 2018

@justahero - i invited you to join the foreman organization, if you make your membership public your PRs will automatically trigger CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.