-
Notifications
You must be signed in to change notification settings - Fork 3
Make servicenow faster - more resilient #51
Conversation
@@ -122,20 +122,6 @@ export class ServiceNowClient { | |||
const response = await this.request({ url }); | |||
const result = response?.data?.result; | |||
|
|||
const redactedResponse = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed log
for (const r of paginatedResponse.result) { | ||
await callback(r); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed log
@@ -13,10 +13,6 @@ export const Steps = { | |||
GROUP_MEMBERS: 'step-group-members', | |||
INCIDENTS: 'step-incidents', | |||
CMDB: 'step-cmdb', | |||
USER_OWNS_CMDB: 'step_user_owns_cmdb', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged this 4 steps into the cmdb step to make the whole process faster
@@ -35,113 +35,80 @@ export async function fetchCMDB( | |||
|
|||
await client.iterateTableResources({ | |||
table: parent, | |||
limit: 3_000, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default for table api is 10_000 so the api is really good at handling large amounts of data. I bumped by x6 to be careful with our memory usage
src/steps/converters.ts
Outdated
@@ -106,7 +107,7 @@ export function createIncidentAssigneeRelationship( | |||
fromType: Entities.INCIDENT._type, | |||
fromKey: incident.sys_id, | |||
toType: Entities.USER._type, | |||
toKey: incident.assigned_to.value, | |||
toKey: incident.assigned_to?.value, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't the toKey
need to always be present?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, I'll remove the optional chaining there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
🚀 PR was released in |
Description
Thank you for contributing to a JupiterOne integration!
Please include a summary of the change and which issue is fixed. Please also
include relevant motivation and context. List any dependencies that are required
for this change.
Summary
Type of change
Please leave any irrelevant options unchecked.
not work as expected)
Checklist
General Development Checklist:
Integration Development Checklist:
Please leave any irrelevant options unchecked.
endpoints, and have documented any additional permissions in
jupiterone.md
, where necessary.API
using
dependsOn
JupiterOne data model
to ensure that any new entities/relationships, and relevant properties,
match the recommended model for this class of data
CHANGELOG.md
file to describe my changesreviewed all existing managed questions referencing the entities,
relationships, and their property names, to ensure those questions still
function with my changes.