[checkpoint_harmony_endpoint] Auth and pagination fixes #12158
Merged
Conversation
This file contains hidden or 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
🚀 Benchmarks reportTo see the full report comment with |
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
💚 Build Succeeded
History
|
|
Contributor
Author
|
Coverage reduction is because I removed all but one of the system tests. Those tests weren't great and were just cut and pasted versions of each other, for data streams that all have identical input config. |
ShourieG
approved these changes
Dec 24, 2024
Contributor
ShourieG
left a comment
There was a problem hiding this comment.
LGTM, tested manually against live endpoint to cross-check proper operation.
|
Package checkpoint_harmony_endpoint - 0.2.1 containing this change is available at https://epr.elastic.co/package/checkpoint_harmony_endpoint/0.2.1/ |
harnish-crest-data
pushed a commit
to chavdaharnish/integrations
that referenced
this pull request
Feb 4, 2025
* First round of fixes.
- Use optional access to cursor values for shorter conditions.
- Assume a 200 response from the auth endpoint, rather trying to set
auth_token from the body of an error response.
- Always get an auth token from cursor data or by fetching a new one,
and then use it for any following request, and...
- Check for the the absence of a task_id to decide when to submit a
query, instead of always doing it after getting the auth token.
- Don't try to get body.message from an error response, because they
actually look like this:
{
"success": false,
"error": {
"status": 400,
"name": "Bad Request",
"details": [
"pageLimit must not be less than 10"
]
}
}
- Clarify the comments describing each section.
* Note the minimum page_limit value.
* Fix the rate limiting variables.
* Handle 401 responses for all requests that use the auth token.
* Save the auth token after any response. Don't clear the auth token at the end of a sequence.
A new token may be fetched in the middle of a sequence, so all results
need to save it.
* Remove unused last_page key from cursor data.
* Clear the task ID when done (query task returned no results).
* Improve clarity in comments and order of handling.
* Remove redundant task_ready key from cursor date and just use page_token instead. Don't override page_token to null when it's already null.
* Keep the token expiry time and get a new token 5 mins before expiry.
* Advance startTime and endTime parameters.
* Extend the default interval and improve variable descriptions.
* System test just one data stream, but make it a better test, that covers polling and pagination.
* Version bump, changelog entry.
harnish-crest-data
pushed a commit
to chavdaharnish/integrations
that referenced
this pull request
Feb 5, 2025
* First round of fixes.
- Use optional access to cursor values for shorter conditions.
- Assume a 200 response from the auth endpoint, rather trying to set
auth_token from the body of an error response.
- Always get an auth token from cursor data or by fetching a new one,
and then use it for any following request, and...
- Check for the the absence of a task_id to decide when to submit a
query, instead of always doing it after getting the auth token.
- Don't try to get body.message from an error response, because they
actually look like this:
{
"success": false,
"error": {
"status": 400,
"name": "Bad Request",
"details": [
"pageLimit must not be less than 10"
]
}
}
- Clarify the comments describing each section.
* Note the minimum page_limit value.
* Fix the rate limiting variables.
* Handle 401 responses for all requests that use the auth token.
* Save the auth token after any response. Don't clear the auth token at the end of a sequence.
A new token may be fetched in the middle of a sequence, so all results
need to save it.
* Remove unused last_page key from cursor data.
* Clear the task ID when done (query task returned no results).
* Improve clarity in comments and order of handling.
* Remove redundant task_ready key from cursor date and just use page_token instead. Don't override page_token to null when it's already null.
* Keep the token expiry time and get a new token 5 mins before expiry.
* Advance startTime and endTime parameters.
* Extend the default interval and improve variable descriptions.
* System test just one data stream, but make it a better test, that covers polling and pagination.
* Version bump, changelog entry.
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Proposed commit message
Tasks to complete the draft
Example auth response:
{"success":true,"data":{"token":"cGibheyJ...","csrf":"a69fe9ac-3764-436c-a254-812a443a6b8d","expires":"Wed, 18 Dec 2024 17:28:31 GMT","expiresIn":1800}}Review tips
Remember that all the
cel.yml.hbsfiles are identical.Can be read commit-by-commit, but it's probably best to focus on the bugfix-related logic in the final version.
Turning off whitespace changes in Github or another diff viewer can help a lot, as many lines have indentation changes.
Checklist
changelog.ymlfile.Related issues