zazuko / cube-creator

A tool to create RDF cubes from CSV files
GNU Affero General Public License v3.0
14 stars 2 forks source link

feat: separate status and icon for canceled jobs #1268

Closed tpluscode closed 2 years ago

tpluscode commented 2 years ago

To properly fix #1218, I figure I would need a separate "canceled" status for jobs neither failed, nor succeeded.

Might as well assign that status to timeout jobs. Especially that I noticed that some "blinking" jobs were actually successful but somehow the update did not register...

changeset-bot[bot] commented 2 years ago

🦋 Changeset detected

Latest commit: ab59de36f3dc6789aef2489232b47c0e76c11a6b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages | Name | Type | | ----------------- | ----- | | @cube-creator/cli | Patch | | @cube-creator/ui | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codecov-commenter commented 2 years ago

Codecov Report

Merging #1268 (4edbf4e) into master (c382d6b) will increase coverage by 2.70%. The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master    #1268      +/-   ##
==========================================
+ Coverage   78.60%   81.30%   +2.70%     
==========================================
  Files         190      190              
  Lines       13286    13290       +4     
  Branches      724      748      +24     
==========================================
+ Hits        10444    10806     +362     
+ Misses       2835     2477     -358     
  Partials        7        7              
Impacted Files Coverage Δ
cli/lib/commands/timeoutJobs.ts 31.34% <0.00%> (ø)
cli/lib/job.ts 74.64% <100.00%> (ø)
cli/lib/log.ts 75.00% <100.00%> (+4.41%) :arrow_up:
packages/core/namespace.ts 100.00% <100.00%> (ø)
cli/lib/commands/import.ts 92.50% <0.00%> (+65.00%) :arrow_up:
cli/lib/import/cubeMetadata.ts 95.20% <0.00%> (+95.20%) :arrow_up:
cli/lib/import/dimensions.ts 97.91% <0.00%> (+97.91%) :arrow_up:
cli/lib/import/cube.ts 100.00% <0.00%> (+100.00%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c382d6b...4edbf4e. Read the comment docs.

tpluscode commented 2 years ago

Not yet. This only adds a new "canceled" status to distinguish them from failed jobs.

Regarding the second part of your question, the fix will only be for publish jobs because we post to one graph. Transform and import always rewrite the entire working copy in cube creator. Not harm done if jobs run in parallel

cristianvasquez commented 2 years ago

I'm not sure I understand how a new state helps to prevent https://github.com/zazuko/cube-creator/issues/1218, which happens with concurrent jobs

Shouldn't we interact with the jobs API directly, using a locking mechanism to handle concurrency?

Something along the lines of: https://docs.gitlab.com/ee/ci/resource_groups/index.html#pipeline-level-concurrency-control-with-cross-projectparent-child-pipelines

cristianvasquez commented 2 years ago

What if it fails to persist the Cancel state?

tpluscode commented 2 years ago

Well, yes, it does not help prevent. This is only an indicator for the UI as to what happened to a job

Thanks for the link, quite interesting but I don't think this helps us.

If other deployment pipelines are running, GitLab waits until those pipelines finish before running another one.

This will not prevent the result of duplicate cubes. It will delay the second publish job but ultimately the cube will be published multiple times. What I we need is to break a pipeline so that it does not write to Lindas

What if it fails to persist the Cancel state?

Nothing. The "timeout jobs" job runs periodically and it will try again at the next attempt

cristianvasquez commented 2 years ago

But wouldn't be ok to show the data of Gitlab pipelines' API directly in the UI? it might provide controls to cancel all kinds of jobs related to publishing in Lindas. Does it make sense?

tpluscode commented 2 years ago

Well, not directly in the UI because of authorisation.

But yea, I like the idea. We send a trigger to GitLab and immediately and get a job URL back. It should be possible to cancel previous jobs at GitLab if someone triggers to publish the same version again. Thanks!