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

'records' in query service stats is wrong #146

Merged
merged 1 commit into from
Apr 2, 2020

Conversation

hvt
Copy link
Contributor

@hvt hvt commented Apr 2, 2020

Description

It uses the rowsInJob instead of rowsInPage inside get_job_results(). get_job_results() only fetches a single page / cursor result, therefore the number of rows in the page must be used.

Motivation and Context

Current records in stats will be number of pages * total number of records in job.

How Has This Been Tested?

Before:

DEBUG:urllib3.connectionpool:https://api.nl.cdl.paloaltonetworks.com:443 "GET /query/v2/jobResults/7d4.....-603d-4a77-946a-bf8e50a....?resultFormat=valuesDictionary HTTP/1.1" 200 21304250
{
  "jobId": "7d4.....-603d-4a77-946a-bf8e50a.....",
  "state": "DONE",
  "resultFormat": "valuesDictionary",
  "rowsInPage": 7932,
  "rowsInJob": 11270,
  "page": {
    "pageCursor": "BH2MYHZ3OE....7767ZKAA======",
    "result": {}
  }
}
DEBUG:urllib3.connectionpool:https://api.nl.cdl.paloaltonetworks.com:443 "GET /query/v2/jobResults/7d4.....-603d-4a77-946a-bf8e50a....?resultFormat=valuesDictionary&pageCursor=BH2MYHZ3OE....7767ZKAA%3D%3D%3D%3D%3D%3D HTTP/1.1" 200 8931523
{
  "jobId": "7d4.....-603d-4a77-946a-bf8e50a....",
  "state": "DONE",
  "resultFormat": "valuesDictionary",
  "rowsInPage": 3338,
  "rowsInJob": 11270,
  "page": {
    "pageCursor": null,
    "result": {}
  }
}
DEBUG:fetch:{'transactions': 3, 'cancel_job': 0, 'create_query': 1, 'get_job': 0, 'list_jobs': 0, 'get_job_results': 2, 'records': 22540}

After:

DEBUG:urllib3.connectionpool:https://api.nl.cdl.paloaltonetworks.com:443 "GET /query/v2/jobResults/dea.....-12eb-4023-8d96-932a834.....?resultFormat=valuesDictionary HTTP/1.1" 200 21304250
{
  "jobId": "dea.....-12eb-4023-8d96-932a834.....",
  "state": "DONE",
  "resultFormat": "valuesDictionary",
  "rowsInPage": 7932,
  "rowsInJob": 11270,
  "page": {
    "pageCursor": "BEPH...7737FIAA====",
    "result": {}
  }
}
DEBUG:urllib3.connectionpool:https://api.nl.cdl.paloaltonetworks.com:443 "GET /query/v2/jobResults/dea.....-12eb-4023-8d96-932a834.....?resultFormat=valuesDictionary&pageCursor=BEPH...7737FIAA%3D%3D%3D%3D HTTP/1.1" 200 8931523
{
  "jobId": "dea.....-12eb-4023-8d96-932a834.....",
  "state": "DONE",
  "resultFormat": "valuesDictionary",
  "rowsInPage": 3338,
  "rowsInJob": 11270,
  "page": {
    "pageCursor": null,
    "result": {}
  }
}
DEBUG:fetch:{'transactions': 3, 'cancel_job': 0, 'create_query': 1, 'get_job': 0, 'list_jobs': 0, 'get_job_results': 2, 'records': 11270}

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes if appropriate.
  • All new and existing tests passed.

It uses the `rowsInJob` instead of `rowsInPage` inside `get_job_results()`. `get_job_results()` only fetches a single page / cursor result, therefore the number of rows in the page must be used.
Copy link
Member

@sserrata sserrata left a comment

Choose a reason for hiding this comment

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

Approved. rowsInJob was indeed being used incorrectly here, partially due to the scrollToken issue preventing the library from iterating past the first page.

@sserrata sserrata merged commit b56308e into PaloAltoNetworks:master Apr 2, 2020
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.

2 participants