-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat: update per API change for importing ruleset #228
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #228 +/- ##
==========================================
+ Coverage 40.86% 41.71% +0.85%
==========================================
Files 81 81
Lines 4860 4921 +61
Branches 111 125 +14
==========================================
+ Hits 1986 2053 +67
+ Misses 2873 2867 -6
Partials 1 1
|
do { | ||
response = await client.sendRequest(options); | ||
if (response.parsedBody?.status === "Succeeded") { | ||
return { isSuccessful: true }; |
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.
the response no need to return?
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.
Just return ApiCenterRulesetImportResult
url: location, | ||
}; | ||
|
||
const timeout = 30000; // 30 seconds in milliseconds |
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.
Do we consider use retry times like 5 times? Or the service team recommend to use timeout?
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.
The 'importing spec' written by service team used this pattern, so I just follow it.
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.
Just synced with Alex yesterday, he recommand to set timeout as 60 seconds, as the job may be long.
No description provided.