Commit 4f8a1c0d authored by Lin Jen-Shin's avatar Lin Jen-Shin

Just use the url from options, not saving it as a column

parent 20dcd522
...@@ -26,10 +26,6 @@ module Ci ...@@ -26,10 +26,6 @@ module Ci
validates :coverage, numericality: true, allow_blank: true validates :coverage, numericality: true, allow_blank: true
validates :ref, presence: true validates :ref, presence: true
validates :environment_url,
length: { maximum: 255 },
allow_nil: true,
addressable_url: true
scope :unstarted, ->() { where(runner_id: nil) } scope :unstarted, ->() { where(runner_id: nil) }
scope :ignore_failures, ->() { where(allow_failure: false) } scope :ignore_failures, ->() { where(allow_failure: false) }
...@@ -147,6 +143,10 @@ module Ci ...@@ -147,6 +143,10 @@ module Ci
environment_url environment_url
end end
def environment_url
options.dig(:environment, :url)
end
def has_environment? def has_environment?
environment.present? environment.present?
end end
......
...@@ -2,8 +2,8 @@ module Ci ...@@ -2,8 +2,8 @@ module Ci
class RetryBuildService < ::BaseService class RetryBuildService < ::BaseService
CLONE_ACCESSORS = %i[pipeline project ref tag options commands name CLONE_ACCESSORS = %i[pipeline project ref tag options commands name
allow_failure stage stage_idx trigger_request allow_failure stage stage_idx trigger_request
yaml_variables when environment environment_url yaml_variables when environment coverage_regex
coverage_regex description tag_list].freeze description tag_list].freeze
def execute(build) def execute(build)
reprocess!(build).tap do |new_build| reprocess!(build).tap do |new_build|
......
class AddEnvironmentUrlToCiBuilds < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column(:ci_builds, :environment_url, :string)
end
end
...@@ -233,7 +233,6 @@ ActiveRecord::Schema.define(version: 20170525174156) do ...@@ -233,7 +233,6 @@ ActiveRecord::Schema.define(version: 20170525174156) do
t.string "coverage_regex" t.string "coverage_regex"
t.integer "auto_canceled_by_id" t.integer "auto_canceled_by_id"
t.boolean "retried" t.boolean "retried"
t.string "environment_url"
end end
add_index "ci_builds", ["auto_canceled_by_id"], name: "index_ci_builds_on_auto_canceled_by_id", using: :btree add_index "ci_builds", ["auto_canceled_by_id"], name: "index_ci_builds_on_auto_canceled_by_id", using: :btree
...@@ -1493,4 +1492,4 @@ ActiveRecord::Schema.define(version: 20170525174156) do ...@@ -1493,4 +1492,4 @@ ActiveRecord::Schema.define(version: 20170525174156) do
add_foreign_key "trending_projects", "projects", on_delete: :cascade add_foreign_key "trending_projects", "projects", on_delete: :cascade
add_foreign_key "u2f_registrations", "users" add_foreign_key "u2f_registrations", "users"
add_foreign_key "web_hook_logs", "web_hooks", on_delete: :cascade add_foreign_key "web_hook_logs", "web_hooks", on_delete: :cascade
end end
\ No newline at end of file
...@@ -61,7 +61,6 @@ module Ci ...@@ -61,7 +61,6 @@ module Ci
allow_failure: job[:ignore], allow_failure: job[:ignore],
when: job[:when] || 'on_success', when: job[:when] || 'on_success',
environment: job[:environment_name], environment: job[:environment_name],
environment_url: job[:environment_url],
coverage_regex: job[:coverage], coverage_regex: job[:coverage],
yaml_variables: yaml_variables(name), yaml_variables: yaml_variables(name),
options: { options: {
......
...@@ -141,7 +141,6 @@ module Gitlab ...@@ -141,7 +141,6 @@ module Gitlab
variables: variables_defined? ? variables_value : nil, variables: variables_defined? ? variables_value : nil,
environment: environment_defined? ? environment_value : nil, environment: environment_defined? ? environment_value : nil,
environment_name: environment_defined? ? environment_value[:name] : nil, environment_name: environment_defined? ? environment_value[:name] : nil,
environment_url: environment_defined? ? environment_value[:url] : nil,
coverage: coverage_defined? ? coverage_value : nil, coverage: coverage_defined? ? coverage_value : nil,
artifacts: artifacts_value, artifacts: artifacts_value,
after_script: after_script_value, after_script: after_script_value,
......
...@@ -63,7 +63,6 @@ FactoryGirl.define do ...@@ -63,7 +63,6 @@ FactoryGirl.define do
trait :teardown_environment do trait :teardown_environment do
environment 'staging' environment 'staging'
environment_url 'http://staging.example.com/$CI_JOB_NAME'
options environment: { name: 'staging', options environment: { name: 'staging',
action: 'stop', action: 'stop',
url: 'http://staging.example.com/$CI_JOB_NAME' } url: 'http://staging.example.com/$CI_JOB_NAME' }
......
...@@ -105,7 +105,6 @@ module Ci ...@@ -105,7 +105,6 @@ module Ci
allow_failure: false, allow_failure: false,
when: "on_success", when: "on_success",
environment: nil, environment: nil,
environment_url: nil,
yaml_variables: [] yaml_variables: []
}) })
end end
...@@ -524,7 +523,6 @@ module Ci ...@@ -524,7 +523,6 @@ module Ci
allow_failure: false, allow_failure: false,
when: "on_success", when: "on_success",
environment: nil, environment: nil,
environment_url: nil,
yaml_variables: [] yaml_variables: []
}) })
end end
...@@ -554,7 +552,6 @@ module Ci ...@@ -554,7 +552,6 @@ module Ci
allow_failure: false, allow_failure: false,
when: "on_success", when: "on_success",
environment: nil, environment: nil,
environment_url: nil,
yaml_variables: [] yaml_variables: []
}) })
end end
...@@ -801,7 +798,6 @@ module Ci ...@@ -801,7 +798,6 @@ module Ci
when: "on_success", when: "on_success",
allow_failure: false, allow_failure: false,
environment: nil, environment: nil,
environment_url: nil,
yaml_variables: [] yaml_variables: []
}) })
end end
...@@ -853,7 +849,6 @@ module Ci ...@@ -853,7 +849,6 @@ module Ci
it 'does return production and URL' do it 'does return production and URL' do
expect(builds.size).to eq(1) expect(builds.size).to eq(1)
expect(builds.first[:environment]).to eq(environment[:name]) expect(builds.first[:environment]).to eq(environment[:name])
expect(builds.first[:environment_url]).to eq(environment[:url])
expect(builds.first[:options]).to include(environment: environment) expect(builds.first[:options]).to include(environment: environment)
end end
...@@ -866,7 +861,6 @@ module Ci ...@@ -866,7 +861,6 @@ module Ci
it 'allows a variable for the port' do it 'allows a variable for the port' do
expect(builds.size).to eq(1) expect(builds.size).to eq(1)
expect(builds.first[:environment]).to eq(environment[:name]) expect(builds.first[:environment]).to eq(environment[:name])
expect(builds.first[:environment_url]).to eq(environment[:url])
expect(builds.first[:options]).to include(environment: environment) expect(builds.first[:options]).to include(environment: environment)
end end
end end
...@@ -1007,7 +1001,6 @@ module Ci ...@@ -1007,7 +1001,6 @@ module Ci
when: "on_success", when: "on_success",
allow_failure: false, allow_failure: false,
environment: nil, environment: nil,
environment_url: nil,
yaml_variables: [] yaml_variables: []
}) })
end end
...@@ -1054,7 +1047,6 @@ module Ci ...@@ -1054,7 +1047,6 @@ module Ci
when: "on_success", when: "on_success",
allow_failure: false, allow_failure: false,
environment: nil, environment: nil,
environment_url: nil,
yaml_variables: [] yaml_variables: []
}) })
expect(subject.second).to eq({ expect(subject.second).to eq({
...@@ -1068,7 +1060,6 @@ module Ci ...@@ -1068,7 +1060,6 @@ module Ci
when: "on_success", when: "on_success",
allow_failure: false, allow_failure: false,
environment: nil, environment: nil,
environment_url: nil,
yaml_variables: [] yaml_variables: []
}) })
end end
......
...@@ -225,7 +225,6 @@ CommitStatus: ...@@ -225,7 +225,6 @@ CommitStatus:
- erased_at - erased_at
- artifacts_expire_at - artifacts_expire_at
- environment - environment
- environment_url
- artifacts_size - artifacts_size
- when - when
- yaml_variables - yaml_variables
......
...@@ -20,7 +20,6 @@ describe Ci::Build, :models do ...@@ -20,7 +20,6 @@ describe Ci::Build, :models do
it { is_expected.to validate_presence_of(:ref) } it { is_expected.to validate_presence_of(:ref) }
it { is_expected.to respond_to(:has_trace?) } it { is_expected.to respond_to(:has_trace?) }
it { is_expected.to respond_to(:trace) } it { is_expected.to respond_to(:trace) }
it { is_expected.to validate_length_of(:environment_url).is_at_most(255) }
describe '#actionize' do describe '#actionize' do
context 'when build is a created' do context 'when build is a created' do
...@@ -435,7 +434,7 @@ describe Ci::Build, :models do ...@@ -435,7 +434,7 @@ describe Ci::Build, :models do
let(:build) do let(:build) do
create(:ci_build, create(:ci_build,
ref: 'master', ref: 'master',
environment_url: 'http://review/$CI_COMMIT_REF_NAME') options: { environment: { url: 'http://review/$CI_COMMIT_REF_NAME' } })
end end
it { is_expected.to eq('http://review/master') } it { is_expected.to eq('http://review/master') }
...@@ -445,7 +444,7 @@ describe Ci::Build, :models do ...@@ -445,7 +444,7 @@ describe Ci::Build, :models do
let(:build) do let(:build) do
create(:ci_build, create(:ci_build,
yaml_variables: [{ key: :APP_HOST, value: 'host' }], yaml_variables: [{ key: :APP_HOST, value: 'host' }],
environment_url: 'http://review/$APP_HOST') options: { environment: { url: 'http://review/$APP_HOST' } })
end end
it { is_expected.to eq('http://review/host') } it { is_expected.to eq('http://review/host') }
...@@ -1244,7 +1243,7 @@ describe Ci::Build, :models do ...@@ -1244,7 +1243,7 @@ describe Ci::Build, :models do
context 'when the URL was set from the job' do context 'when the URL was set from the job' do
before do before do
build.update(environment_url: 'http://host/$CI_JOB_NAME') build.update(options: { environment: { url: 'http://host/$CI_JOB_NAME' } })
end end
it_behaves_like 'containing environment variables' it_behaves_like 'containing environment variables'
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment