diff options
| -rw-r--r-- | app/assets/javascripts/jobs/components/sidebar_details_block.vue | 8 | ||||
| -rw-r--r-- | app/models/ci/build.rb | 18 | ||||
| -rw-r--r-- | app/models/ci/build_metadata.rb | 25 | ||||
| -rw-r--r-- | app/presenters/ci/build_metadata_presenter.rb | 20 | ||||
| -rw-r--r-- | app/presenters/ci/build_presenter.rb | 13 | ||||
| -rw-r--r-- | app/serializers/build_details_entity.rb | 5 | ||||
| -rw-r--r-- | app/serializers/build_metadata_entity.rb | 10 | ||||
| -rw-r--r-- | db/migrate/20180221022556_add_used_timeout_and_timeout_source_columns_to_ci_builds.rb | 10 | ||||
| -rw-r--r-- | db/migrate/20180301010859_create_ci_builds_metadata_table.rb | 13 | ||||
| -rw-r--r-- | db/migrate/20180301011323_add_build_foreign_key_to_ci_builds_metadata.rb | 15 | ||||
| -rw-r--r-- | db/schema.rb | 11 | ||||
| -rw-r--r-- | spec/javascripts/jobs/mock_data.js | 6 | ||||
| -rw-r--r-- | spec/models/ci/build_spec.rb | 4 | ||||
| -rw-r--r-- | spec/models/concerns/chronic_duration_attribute_spec.rb | 2 | ||||
| -rw-r--r-- | spec/services/ci/retry_build_service_spec.rb | 2 |
15 files changed, 110 insertions, 52 deletions
diff --git a/app/assets/javascripts/jobs/components/sidebar_details_block.vue b/app/assets/javascripts/jobs/components/sidebar_details_block.vue index ad859679a1e..15584922d1f 100644 --- a/app/assets/javascripts/jobs/components/sidebar_details_block.vue +++ b/app/assets/javascripts/jobs/components/sidebar_details_block.vue @@ -45,10 +45,10 @@ return `#${this.job.runner.id}`; }, timeout() { - let t = `${this.job.timeout.value}`; + let t = `${this.job.metadata.used_timeout_human_readable}`; - if (this.job.timeout.source != null) { - t += ` (from ${this.job.timeout.source})`; + if (this.job.metadata.timeout_source != null) { + t += ` (from ${this.job.metadata.timeout_source})`; } return t; @@ -130,7 +130,7 @@ /> <detail-row class="js-job-timeout" - v-if="job.timeout" + v-if="job.metadata.used_timeout_human_readable" title="Timeout" :help-url="runnerHelpUrl" :value="timeout" diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index cbd2cc6c58f..4b4a988d600 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -6,7 +6,6 @@ module Ci include ObjectStorage::BackgroundMove include Presentable include Importable - include ChronicDurationAttribute MissingDependenciesError = Class.new(StandardError) @@ -25,6 +24,8 @@ module Ci has_one :job_artifacts_metadata, -> { where(file_type: Ci::JobArtifact.file_types[:metadata]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id has_one :job_artifacts_trace, -> { where(file_type: Ci::JobArtifact.file_types[:trace]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id + has_one :metadata, class_name: 'Ci::BuildMetadata' + # The "environment" field for builds is a String, and is the unexpanded name def persisted_environment @persisted_environment ||= Environment.find_by( @@ -84,6 +85,10 @@ module Ci before_save :ensure_token before_destroy { unscoped_project } + before_create do |build| + build.metadata = Ci::BuildMetadata.new + end + after_create unless: :importing? do |build| run_after_commit { BuildHooksWorker.perform_async(build.id) } end @@ -91,14 +96,6 @@ module Ci after_commit :update_project_statistics_after_save, on: [:create, :update] after_commit :update_project_statistics, on: :destroy - chronic_duration_attr_reader :used_timeout_human_readable, :used_timeout - - enum timeout_source: { - unknown_timeout_source: nil, - project_timeout_source: 1, - runner_timeout_source: 2 - } - class << self # This is needed for url_for to work, # as the controller is JobsController @@ -164,8 +161,7 @@ module Ci end before_transition pending: :running do |build| - build.used_timeout = build.timeout - build.timeout_source = build.timeout < build.project.build_timeout ? :runner_timeout_source : :project_timeout_source + build.metadata.save_timeout_state! end end diff --git a/app/models/ci/build_metadata.rb b/app/models/ci/build_metadata.rb new file mode 100644 index 00000000000..7a1315dfcf9 --- /dev/null +++ b/app/models/ci/build_metadata.rb @@ -0,0 +1,25 @@ +module Ci + class BuildMetadata < ActiveRecord::Base + extend Gitlab::Ci::Model + include Presentable + include ChronicDurationAttribute + + self.table_name = 'ci_builds_metadata' + + belongs_to :build, class_name: 'Ci::Build' + + chronic_duration_attr_reader :used_timeout_human_readable, :used_timeout + + enum timeout_source: { + unknown_timeout_source: nil, + project_timeout_source: 1, + runner_timeout_source: 2 + } + + def save_timeout_state! + self.used_timeout = build.timeout + self.timeout_source = build.timeout < build.project.build_timeout ? :runner_timeout_source : :project_timeout_source + save! + end + end +end diff --git a/app/presenters/ci/build_metadata_presenter.rb b/app/presenters/ci/build_metadata_presenter.rb new file mode 100644 index 00000000000..e80eb19ea19 --- /dev/null +++ b/app/presenters/ci/build_metadata_presenter.rb @@ -0,0 +1,20 @@ +module Ci + class BuildMetadataPresenter < Gitlab::View::Presenter::Delegated + + TIMEOUT_SOURCES = { + unknown_timeout_source: nil, + project_timeout_source: 'project', + runner_timeout_source: 'runner' + }.freeze + + presents :metadata + + def timeout_source + return unless metadata.timeout_source? + + TIMEOUT_SOURCES[metadata.timeout_source.to_sym] || + metadata.timeout_source + end + + end +end diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb index be6cc2e70b1..9345e5069bc 100644 --- a/app/presenters/ci/build_presenter.rb +++ b/app/presenters/ci/build_presenter.rb @@ -1,12 +1,6 @@ module Ci class BuildPresenter < Gitlab::View::Presenter::Delegated - TIMEOUT_SOURCES = { - unknown_timeout_source: nil, - project_timeout_source: 'project', - runner_timeout_source: 'runner' - }.freeze - presents :build def erased_by_user? @@ -25,13 +19,6 @@ module Ci end end - def timeout_source - return unless build.timeout_source? - - TIMEOUT_SOURCES[build.timeout_source.to_sym] || - build.timeout_source - end - def trigger_variables return [] unless trigger_request diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index 0ffc537dfd8..ca4480fe2b1 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -5,10 +5,7 @@ class BuildDetailsEntity < JobEntity expose :runner, using: RunnerEntity expose :pipeline, using: PipelineEntity - expose :timeout, if: -> (*) { !build.used_timeout.nil? } do |build| - { value: build.used_timeout_human_readable, - source: build.present.timeout_source } - end + expose :metadata, using: BuildMetadataEntity expose :erased_by, if: -> (*) { build.erased? }, using: UserEntity expose :erase_path, if: -> (*) { build.erasable? && can?(current_user, :erase_build, build) } do |build| diff --git a/app/serializers/build_metadata_entity.rb b/app/serializers/build_metadata_entity.rb new file mode 100644 index 00000000000..71c7295ba9f --- /dev/null +++ b/app/serializers/build_metadata_entity.rb @@ -0,0 +1,10 @@ +class BuildMetadataEntity < Grape::Entity + + expose :used_timeout_human_readable do |metadata| + metadata.used_timeout_human_readable unless metadata.used_timeout.nil? + end + + expose :timeout_source do |metadata| + metadata.present.timeout_source + end +end diff --git a/db/migrate/20180221022556_add_used_timeout_and_timeout_source_columns_to_ci_builds.rb b/db/migrate/20180221022556_add_used_timeout_and_timeout_source_columns_to_ci_builds.rb deleted file mode 100644 index 18c4fd5bae4..00000000000 --- a/db/migrate/20180221022556_add_used_timeout_and_timeout_source_columns_to_ci_builds.rb +++ /dev/null @@ -1,10 +0,0 @@ -class AddUsedTimeoutAndTimeoutSourceColumnsToCiBuilds < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - def change - add_column :ci_builds, :used_timeout, :integer - add_column :ci_builds, :timeout_source, :integer - end -end diff --git a/db/migrate/20180301010859_create_ci_builds_metadata_table.rb b/db/migrate/20180301010859_create_ci_builds_metadata_table.rb new file mode 100644 index 00000000000..9d5e9c1779b --- /dev/null +++ b/db/migrate/20180301010859_create_ci_builds_metadata_table.rb @@ -0,0 +1,13 @@ +class CreateCiBuildsMetadataTable < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + create_table :ci_builds_metadata do |t| + t.integer :build_id, null: false + t.integer :used_timeout + t.integer :timeout_source + end + end +end diff --git a/db/migrate/20180301011323_add_build_foreign_key_to_ci_builds_metadata.rb b/db/migrate/20180301011323_add_build_foreign_key_to_ci_builds_metadata.rb new file mode 100644 index 00000000000..feda2d6e9c9 --- /dev/null +++ b/db/migrate/20180301011323_add_build_foreign_key_to_ci_builds_metadata.rb @@ -0,0 +1,15 @@ +class AddBuildForeignKeyToCiBuildsMetadata < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_foreign_key(:ci_builds_metadata, :ci_builds, column: :build_id) + end + + def down + remove_foreign_key(:ci_builds_metadata, column: :build_id) + end +end diff --git a/db/schema.rb b/db/schema.rb index 9b126385045..5463b3f1219 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -311,8 +311,6 @@ ActiveRecord::Schema.define(version: 20180327101207) do t.integer "artifacts_metadata_store" t.boolean "protected" t.integer "failure_reason" - t.integer "used_timeout" - t.integer "timeout_source" end add_index "ci_builds", ["artifacts_expire_at"], name: "index_ci_builds_on_artifacts_expire_at", where: "(artifacts_file <> ''::text)", using: :btree @@ -331,6 +329,12 @@ ActiveRecord::Schema.define(version: 20180327101207) do add_index "ci_builds", ["updated_at"], name: "index_ci_builds_on_updated_at", using: :btree add_index "ci_builds", ["user_id"], name: "index_ci_builds_on_user_id", using: :btree + create_table "ci_builds_metadata", force: :cascade do |t| + t.integer "build_id", null: false + t.integer "used_timeout" + t.integer "timeout_source" + end + create_table "ci_group_variables", force: :cascade do |t| t.string "key", null: false t.text "value" @@ -460,8 +464,8 @@ ActiveRecord::Schema.define(version: 20180327101207) do t.boolean "run_untagged", default: true, null: false t.boolean "locked", default: false, null: false t.integer "access_level", default: 0, null: false - t.string "ip_address" t.integer "maximum_job_timeout" + t.string "ip_address" end add_index "ci_runners", ["contacted_at"], name: "index_ci_runners_on_contacted_at", using: :btree @@ -2031,6 +2035,7 @@ ActiveRecord::Schema.define(version: 20180327101207) do add_foreign_key "ci_builds", "ci_pipelines", column: "auto_canceled_by_id", name: "fk_a2141b1522", on_delete: :nullify add_foreign_key "ci_builds", "ci_stages", column: "stage_id", name: "fk_3a9eaa254d", on_delete: :cascade add_foreign_key "ci_builds", "projects", name: "fk_befce0568a", on_delete: :cascade + add_foreign_key "ci_builds_metadata", "ci_builds", column: "build_id", name: "fk_e20479742e", on_delete: :cascade add_foreign_key "ci_group_variables", "namespaces", column: "group_id", name: "fk_33ae4d58d8", on_delete: :cascade add_foreign_key "ci_job_artifacts", "ci_builds", column: "job_id", on_delete: :cascade add_foreign_key "ci_job_artifacts", "projects", on_delete: :cascade diff --git a/spec/javascripts/jobs/mock_data.js b/spec/javascripts/jobs/mock_data.js index 6c3f39f0193..8a7ba50830c 100644 --- a/spec/javascripts/jobs/mock_data.js +++ b/spec/javascripts/jobs/mock_data.js @@ -115,9 +115,9 @@ export default { commit_path: '/root/ci-mock/commit/c58647773a6b5faf066d4ad6ff2c9fbba5f180f6', }, }, - timeout: { - value: '1m 40s', - source: 'runner', + metadata: { + used_timeout_human_readable: '1m 40s', + timeout_source: 'runner', }, merge_request: { iid: 2, diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index c3624158b76..1da84d6caa9 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2047,11 +2047,11 @@ describe Ci::Build do shared_examples 'saves data on transition' do it 'saves used_timeout' do - expect { job.run! }.to change { job.reload.used_timeout }.from(nil).to(expected_timeout) + expect { job.run! }.to change { job.reload.metadata.used_timeout }.from(nil).to(expected_timeout) end it 'saves timeout_source' do - expect { job.run! }.to change { job.reload.timeout_source }.from('unknown_timeout_source').to(expected_timeout_source) + expect { job.run! }.to change { job.reload.metadata.timeout_source }.from('unknown_timeout_source').to(expected_timeout_source) end end diff --git a/spec/models/concerns/chronic_duration_attribute_spec.rb b/spec/models/concerns/chronic_duration_attribute_spec.rb index 31db7d055cc..fbbbcd374ee 100644 --- a/spec/models/concerns/chronic_duration_attribute_spec.rb +++ b/spec/models/concerns/chronic_duration_attribute_spec.rb @@ -48,7 +48,7 @@ end describe 'ChronicDurationAttribute - reader' do let(:source_field) {:used_timeout} let(:virtual_field) {:used_timeout_human_readable} - subject {Ci::Build.new} + subject {Ci::BuildMetadata.new} it "doesn't contain dynamically created writer method" do expect(subject.class).not_to be_public_method_defined("#{virtual_field}=") diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index e425e80e51e..8de0bdf92e2 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -30,7 +30,7 @@ describe Ci::RetryBuildService do runner_id tag_taggings taggings tags trigger_request_id user_id auto_canceled_by_id retried failure_reason artifacts_file_store artifacts_metadata_store - used_timeout timeout_source].freeze + metadata].freeze shared_examples 'build duplication' do let(:another_pipeline) { create(:ci_empty_pipeline, project: project) } |