Bug job id return in gitlab host connector #1604

oliviermgx · 2024-06-07T10:26:36Z

No description provided.

lexoyo

Je pense qu'il faut refactorer un peu l'algo, je t'ai mis qques pistes pour l'améliorer

src/ts/plugins/server/GitlabHostingConnector.ts

lexoyo · 2024-06-07T14:35:52Z

src/ts/plugins/server/GitlabHostingConnector.ts

+    let i = 0
+    setTimeout (() => {
+      while (jobs[0].ref !== tag && i<20) {
+        jobs = this.callApi(session, `api/v4/projects/${websiteId}/jobs`, 'GET')


Although there's a condition to limit the loop to 20 iterations, the loop does not include any delay or await between iterations. It will make many calls to callApi for each timeout

I don't think so, the setTimeout is tune with a delay and I have mesured the mean number f call that are between 0 and 5 cycles. What I am missing ?

you don't wait for an answer (await)
so jobs will be a promise, not the actual result
and it will run 100 times without waiting for a result

just to verify my comprehension, this should work better ?
setTimeout (async () => { while (jobs[0].ref !== tag && i<20) { jobs = await this.callApi(session,api/v4/projects/${websiteId}/jobs, 'GET') i++ } }, 100)

no because the setTimeout will not wait that the async finishes, you have multiple calls in parallel

check my proposal of algorithm, this will do a call, wait for the response, wait a little, loop

check my proposal of algorithm, this will do a call, wait for the response, wait a little, loop

your algo will do: wait a little, start a call but don't get the result and don't wait, loop

sorry for my lack of skills in async js field. I will try again !

lexoyo · 2024-06-07T14:40:17Z

src/ts/plugins/server/GitlabHostingConnector.ts

-    const jobs = await this.callApi(session, `api/v4/projects/${websiteId}/jobs`, 'GET')
-    return `${projectUrl}/-/jobs/${jobs[0].id}`
+  async getGitlabJobLogsUrl(session: GitlabSession, websiteId: WebsiteId, job: PublicationJobData, { startJob, jobSuccess, jobError }: JobManager, projectUrl: string, tag): Promise<string> {
+    let jobs = await this.callApi(session, `api/v4/projects/${websiteId}/jobs`, 'GET')


Don't make a first call then many other ones

I suggest :

Do

await get the tag (callApi)

if the tag is the expected one return it

await TIME_BEFORE_RETRY - see the way to do this (first solution of this article)

While elapsed time < TIMEOUT

EDIT: I changed it a little

"Don't make a first call then many other ones"
I don't understand, I need to get the first time the jobs in order to read its jobs.ref containing the current tag ? otherwise I don't know how to initialize the while loop ?

"await TIME_BEFORE_RETRY ", I thought that setTimeout do the same ? https://www.freecodecamp.org/news/javascript-settimeout-how-to-set-a-timer-in-javascript-or-sleep-for-n-seconds/

"await get the tag (callApi)
await TIME_BEFORE_RETRY - see the way to do this (first solution of this article)
While elapsed time < TIMEOUT and the tag is not the expected one"

I beleived I'm doing exactly this algo ?

"Don't make a first call then many other ones"

I don't understand, I need to get the first time the jobs in order to read its jobs.ref containing the current tag ? otherwise I don't know how to initialize the while loop ?

use a do while instead of a while

"await TIME_BEFORE_RETRY ", I thought that setTimeout do the same ? https://www.freecodecamp.org/news/javascript-settimeout-how-to-set-a-timer-in-javascript-or-sleep-for-n-seconds/

the setTimeout takes a callback, and given the complexity of this algorithm you should use await and no callback, it will make it obvious my other feedback

thank you for your time to explain to me

I will change with do while, thanks for advices

lexoyo · 2024-06-12T15:22:29Z

src/ts/plugins/server/GitlabHostingConnector.ts

+      const jobs = await this.callApi(session, `api/v4/projects/${websiteId}/jobs`, 'GET')
+      if (jobs[0].ref === tag) {return `${projectUrl}/-/jobs/${jobs[0].id}`}
+      await setTimeout(5)
+    } while (i<19)


i think it would be clearer to compare execution time instead of the number of loops

use Date.now() to get the current time

lexoyo · 2024-06-12T15:23:14Z

src/ts/plugins/server/GitlabHostingConnector.ts

+      i++
+      const jobs = await this.callApi(session, `api/v4/projects/${websiteId}/jobs`, 'GET')
+      if (jobs[0].ref === tag) {return `${projectUrl}/-/jobs/${jobs[0].id}`}
+      await setTimeout(5)


5 here is in milliseconds, maybe you should give it more time?

I could but I have tested and it works between 1 and less than 10 * 5 ms ? I could put 50 ms if it's better ? else ?

lexoyo · 2024-06-14T10:49:15Z

src/ts/plugins/server/GitlabHostingConnector.ts

+      const jobs = await this.callApi(session, `api/v4/projects/${websiteId}/jobs`, 'GET')
+      if (jobs[0].ref === tag) {return `${projectUrl}/-/jobs/${jobs[0].id}`}
+      await setTimeout(5000)
+    } while ((Date.now() - t0) < 15000)


laaaast thing, you may want to set this timeout as an option?

lexoyo

Great! Well done!!

* gitlab hosting connector with good current job id return * gitlab hosting connector with good current job id return 2 * gitlab hosting connector with good current job id return v3 * with setTimout problem corrected * with setTimout problem corrected 2 * with setTimout problem corrected 3 * with setTimout problem corrected 3 * with setTimout problem corrected 3 * with setTimout problem corrected 4 * with setTimout problem corrected 4 * with setTimout problem corrected 5 * with setTimout problem corrected 5 * with setTimout problem corrected 6 * with timer in variables --------- Co-authored-by: Ubuntu <ubuntu@ov-bddc60.infomaniak.ch>

* Added plugin * Update package.json * The publication dialog now displays a message to suggest the user to verify their GitLab account when needed + Loading bar visual improvement * Update grapesjs-plugins.scss * Update GitlabHostingConnector.ts * Added a target=_blank on the link * Update package.json * Bug job id return in gitlab host connector (#1604) * gitlab hosting connector with good current job id return * gitlab hosting connector with good current job id return 2 * gitlab hosting connector with good current job id return v3 * with setTimout problem corrected * with setTimout problem corrected 2 * with setTimout problem corrected 3 * with setTimout problem corrected 3 * with setTimout problem corrected 3 * with setTimout problem corrected 4 * with setTimout problem corrected 4 * with setTimout problem corrected 5 * with setTimout problem corrected 5 * with setTimout problem corrected 6 * with timer in variables --------- Co-authored-by: Ubuntu <ubuntu@ov-bddc60.infomaniak.ch> * The publication dialog now displays a message to suggest the user to verify their GitLab account when needed + Loading bar visual improvement * Merged changes * Fixed bad merge --------- Co-authored-by: oliviermgx <73127173+oliviermgx@users.noreply.github.com> Co-authored-by: Ubuntu <ubuntu@ov-bddc60.infomaniak.ch>

lexoyo requested changes Jun 7, 2024

View reviewed changes

oliviermgx requested a review from lexoyo June 12, 2024 12:28

lexoyo requested changes Jun 12, 2024

View reviewed changes

Ubuntu added 13 commits June 13, 2024 15:41

gitlab hosting connector with good current job id return

9079f4b

gitlab hosting connector with good current job id return 2

972765f

gitlab hosting connector with good current job id return v3

7e440cc

with setTimout problem corrected

8bdf893

with setTimout problem corrected 2

a845e08

with setTimout problem corrected 3

84ca8ec

with setTimout problem corrected 3

5176792

with setTimout problem corrected 3

c0ce07d

with setTimout problem corrected 4

5bd9085

with setTimout problem corrected 4

1488bd8

with setTimout problem corrected 5

c1c292f

with setTimout problem corrected 5

8258b01

with setTimout problem corrected 6

f95a2ea

oliviermgx force-pushed the bug_job_id branch from 8cb609e to f95a2ea Compare June 14, 2024 07:33

oliviermgx requested a review from lexoyo June 14, 2024 07:36

lexoyo requested changes Jun 14, 2024

View reviewed changes

with timer in variables

4069221

oliviermgx requested a review from lexoyo June 16, 2024 17:12

lexoyo approved these changes Jun 19, 2024

View reviewed changes

lexoyo merged commit a2cae53 into silexlabs:dev Jun 19, 2024
2 checks passed

oliviermgx deleted the bug_job_id branch August 13, 2024 13:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug job id return in gitlab host connector #1604

Bug job id return in gitlab host connector #1604

oliviermgx commented Jun 7, 2024

lexoyo left a comment

lexoyo Jun 7, 2024

oliviermgx Jun 7, 2024

lexoyo Jun 11, 2024

oliviermgx Jun 11, 2024

lexoyo Jun 11, 2024

lexoyo Jun 11, 2024

oliviermgx Jun 11, 2024

lexoyo Jun 7, 2024 •

edited

Loading

oliviermgx Jun 7, 2024

oliviermgx Jun 7, 2024

oliviermgx Jun 7, 2024

lexoyo Jun 11, 2024

lexoyo Jun 11, 2024

oliviermgx Jun 11, 2024

oliviermgx Jun 11, 2024

lexoyo Jun 12, 2024

lexoyo Jun 12, 2024

oliviermgx Jun 12, 2024

lexoyo Jun 14, 2024

lexoyo left a comment

Bug job id return in gitlab host connector #1604

Bug job id return in gitlab host connector #1604

Conversation

oliviermgx commented Jun 7, 2024

lexoyo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lexoyo Jun 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lexoyo left a comment

Choose a reason for hiding this comment

lexoyo Jun 7, 2024 •

edited

Loading