Implement Commit Status API
This is preliminary implementation of Commit Status API, pretty much compatible with GitHub.
- The Commit Statuses are stored in separate table: ci_commit_status.
- The POST inserts a new row.
- To POST execute GitLab API
post :id/repository/commits/:sha/status
. This accepts dual authorization:
- Using authorized user
- Using ci-token to allow easy posting from CI Services
- This adds predefined variable to GitLab CI build environment: CI_BUILD_STATUS_URL, allowing to easy post status from within build (ex. with code coverage or other metrics).
- This adds statuses to commit's builds view.
- The commit's status is calculated taking into account status of all builds and all posted statuses.
- The commit statuses doesn't trigger notifications.
- The commit status API introduces two new privileges:
read_commit_statuses
andcreate_commit_status
. - We still miss a few tests and documentation updates for API and CI.
@dzaporozhets @sytses What do you think?
Merge request reports
Activity
Added 1 commit:
- c7b90db4 - Fix API
- Edited by Kamil Trzcińśki
- app/models/commit_status.rb 0 → 100644
43 end 44 45 def finished_at 46 @created_at ||= created_at if complete? 47 end 48 49 def duration 50 if started? && complete? 51 finished_at - started_at 52 end 53 end 54 55 def the_same 56 CommitStatus.where(commit_id: commit_id, context: context, ref: ref) 57 end 58 end @ayufan this class has too much common with Ci::Build. Are you still sure we want to keep same data in 2 different classes?
Ci::Build is used in many other contexts where CommitStatus doesn't fit: ex. runners.
I can think that Ci::CommitStatus may be a base for Ci::Build. I kind of against renaming Ci::Build, because there's too much specific logic. I can also think of option where Ci::Build creates a new Ci::CommitStatus on status change.
Secondly: each post currently creates separate CommitStatus entry, it doesn't update existing one for the same ref and context. I did that delibirately.
Edited by Kamil TrzcińśkiI can also think of option where Ci::Build creates a new Ci::CommitStatus on status change.
I think it makes sense to use commit statuses ourselves as well, just like we expect other CI systems to.
each post currently creates separate CommitStatus entry, it doesn't update existing one for the same ref and context.
That's what GitHub does as well right?
I think it makes sense to use commit statuses ourselves as well, just like we expect other CI systems to.
Yes, I thought that too.
That's what GitHub does as well right?
Yes, they keep track of all posted statuses. It makes sense, because this is something that comes from outside.
CommitStatus is stateless object, that behaves more like an event.
@ayufan does this implement https://gitlab.com/gitlab-org/gitlab-ce/issues/2595 ?
Can you comment there what the json payload is?
@sytse Yes
The
:id/repository/commits/:sha/status
accepts POST with:# id (required) - The ID of a project # sha (required) - The commit hash # ref (optional) - The ref # state (required) - The state of the status. Can be: pending, running, success, error or failure # target_url (optional) - The target URL to associate with this status # description (optional) - A short description of the status # context (optional) - A string label to differentiate this status from the status of other systems. Default: "default"
Edited by Kamil Trzcińśki@ayufan Thanks! I asked for a comment
there
so I pasted it to the issue and propose we discuss there since there is more context there.These are model parameters for Ci::Build and Ci::CommitStatus:
create_table "ci_builds", force: true do |t| t.integer "commit_id" t.string "ref" t.boolean "tag" t.string "stage" t.string "status" t.datetime "finished_at" t.text "trace" t.datetime "started_at" t.integer "runner_id" t.float "coverage" t.text "commands" t.string "name" t.boolean "deploy", default: false t.text "options" t.boolean "allow_failure", default: false, null: false t.integer "trigger_request_id" t.timestamps end create_table :ci_commit_statuses, force: true do |t| t.integer :commit_id t.string :ref t.string :status t.string :context, default: 'default' t.string :target_url t.string :description t.timestamps end
There's very small overlap in actual data.
@ayufan to me it feels like the same thing, but I leave it up to @dzaporozhets and you.
Added 34 commits:
-
c7b90db4...1f11096c - 24 commits from branch
master
- d4bb91e0 - Fix API (+7 squashed commits)
- f66736c8 - Expose Commit Status in RepoCommitDetail
- 17d55a4f - Create CommitStatus specs
- 6f4868f0 - Remove CI_BUILD_STATUS_URL
- 434f18eb - Move skip_ci check to CreateCommitService
- 263dc183 - Make Ci::Build to inherit from CommitStatus and add ExternalStatus
- 8cbf143f - Fix me
- 112de450 - WIP
- 8f968ded - Fix tests
- f70bf230 - Revert some changes
Toggle commit list-
c7b90db4...1f11096c - 24 commits from branch
@dzaporozhets I followed your guide:
- We have CommitStatus
- We have Ci::Build which inherits from CommitStatus
- I introduced ExternalStatus (which should be named ExternalCommitStatus) which inherits from CommitStatus and is created for all external POSTs
Tests are broken, but more or less this is how it will finally look like.
@ayufan Awesome!
@ayufan Having Ci::Build use CommitStatus wasn't an option?
I'd rather have everything work around generic commit statuses, than have special cases for GitLab CI vs the rest.
@dzaporozhets @DouweM is talking about leaving the Ci::Build and CommitStatus in separate tables as it was before, but instead of fuzzing status in Ci::Commit is to create a new CommitStatus on every Ci::Build status change, basically "posting" Ci::Build status to CommitStatus. This was also one of my previous propositions :)
The idea is that GitLab CI would function just like any other CI, posting commit statuses into the system. Would keep code cleaner because we wouldn't have any (or at least less) GitLab CI / other CI, Ci::Build / ExternalCommitStatus branching in logic or UI.
Edited by Douwe Maan