metric rollups api #4
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Aug 11, 2017
added 4 commits
August 14, 2017 10:52
…2 years of daily metrics uset default interval
|
@miq-bot remove_label wip |
|
could you update the PR description to mention dates in the API supported form (YYYY-MM-DD & YYYY-MM-DDTHH:MM:SSZ) |
|
LGTM!! Thanks @jntullo, minor changes needed. |
fix identifier
|
Checked commits jntullo/manageiq-api@74ea576~...b79642d with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
Thanks @jntullo for the update. LGTM!! Please have a follow-up PR for the codeclimate issue, thanks. |
Merged
|
Hi @jntullo we will need all these parameters supported by the cli, exe/manageiq-api. Thanks. |
NickLaMuro
added a commit
to NickLaMuro/manageiq-api
that referenced
this pull request
Dec 5, 2018
The fix here simply matches up the `if` checks used in: - `Api::ReportsController#reports_search_conditions` - `MiqReport.for_user` to both use `User#report_admin_user?` * * * Further background info: This bug is the result of a collision between the following two PRs: - ManageIQ/manageiq#15472 - ManageIQ/manageiq#17444 The first PR, ManageIQ/manageiq#15472, introduces: `Api::ReportsController#reports_search_conditions` Which uses `unless User.current_user.admin?` to check if it should use the `where_clause` from the `MiqReport.for_user` for the search conditions (passed into a `ActiveRecord` `.where` farther up the stack). The idea for this initial check is building this where clause only makes sense if it isn't an `all`, otherwise where it is used will cause an error in Postgres syntax because of `... WHERE ()` being passed This, however, doesn't match the `user.admin_user?`(at the time of ManageIQ/manageiq#15472) check that is done in `.for_user` method, which means it would be called for users that don't match the `admin` username, but still are part of either of the following roles - `"EvmRole-super_administrator"` - `"EvmRole-administrator"` The bug in question: https://bugzilla.redhat.com/show_bug.cgi?id=1650531 tested this with a custom role, which doesn't match either of those two roles, so the `User#admin?` check and `User#admin_user?` check happened to be equivalent in `5.9` (see comment ManageIQ#4 in BZ link), but not `5.10`. But, if the replication steps had instead said to: - Create a user, "User1", that is part of the "EvmGroup-administrator" group This would have also failed in the same way on `5.9` as it does on `5.10` currently. This fails on 5.10, though, because in ManageIQ/manageiq#17444, the `admin_user?` method changes to `report_admin_user?`, and it's functionality also now checks if the role is valid for querying the group role's product features the user has access to, and it only has an eject for the role that happens to be `admin?`. Meaning that the `admin` "super user" still has no problem with this query, but any other user that could still count as a `report_admin_user?` would (both users in the `EvmGroup-administrator` role and just having the proper features enabled)
NickLaMuro
added a commit
to NickLaMuro/manageiq-api
that referenced
this pull request
Dec 5, 2018
The fix here simply matches up the `if` checks used in: - `Api::ReportsController#reports_search_conditions` - `MiqReport.for_user` to both use `User#report_admin_user?` * * * Further background info: This bug is the result of a collision between the following two PRs: - ManageIQ/manageiq#15472 - ManageIQ/manageiq#17444 The first PR, ManageIQ/manageiq#15472, introduces: `Api::ReportsController#reports_search_conditions` Which uses `unless User.current_user.admin?` to check if it should use the `where_clause` from the `MiqReport.for_user` for the search conditions (passed into a `ActiveRecord` `.where` farther up the stack). The idea for this initial check is building this where clause only makes sense if it isn't an `all`, otherwise where it is used will cause an error in Postgres syntax because of `... WHERE ()` being passed This, however, doesn't match the `user.admin_user?`(at the time of ManageIQ/manageiq#15472) check that is done in `.for_user` method, which means it would be called for users that don't match the `admin` username, but still are part of either of the following roles - `"EvmRole-super_administrator"` - `"EvmRole-administrator"` The bug in question: https://bugzilla.redhat.com/show_bug.cgi?id=1650531 tested this with a custom role, which doesn't match either of those two roles, so the `User#admin?` check and `User#admin_user?` check happened to be equivalent in `5.9` (see comment ManageIQ#4 in BZ link), but not `5.10`. But, if the replication steps had instead said to: - Create a user, "User1", that is part of the "EvmGroup-administrator" group This would have also failed in the same way on `5.9` as it does on `5.10` currently. This fails on 5.10, though, because in ManageIQ/manageiq#17444, the `admin_user?` method changes to `report_admin_user?`, and it's functionality also now checks if the role is valid for querying the group role's product features the user has access to, and it only has an eject for the role that happens to be `admin?`. Meaning that the `admin` "super user" still has no problem with this query, but any other user that could still count as a `report_admin_user?` would (both users in the `EvmGroup-administrator` role and just having the proper features enabled) Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1650531
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dependent on: ManageIQ/manageiq#15549
Metric rollups API requires the following parameters:
Optional parameters:
Paging
limitto override)Examples
Minimum attributes:
GET /api/metric_rollups?resource_type=VmOrTemplate&resource_ids=[1,2,3]&capture_interval=hourly&start_date='2017-08-22 08:57:52 -0400'End date:
GET /api/metric_rollups?resource_type=VmOrTemplate&resource_ids=[1,2,3]&capture_interval=hourly&start_date=2017-08-01&end_date=2017-08-22Limit:
GET /api/metric_rollups?resource_type=VmOrTemplate&resource_ids=[1,2,3]&capture_interval=hourly&start_date=2017-08-01&end_date=2017-08-22&limit=2000Dates are in the format:
YYYY-MM-DDorYYYY-MM-DDTHH:MM:SSZ@miq_bot add_label wip, api
@miq-bot assign @abellotti