Skip to content

Bug 1625451 - Provide default for application field#6213

Merged
ionutgoldan merged 1 commit intomozilla:masterfrom
octavian-negru:provide-default-for-application-field-2
Apr 3, 2020
Merged

Bug 1625451 - Provide default for application field#6213
ionutgoldan merged 1 commit intomozilla:masterfrom
octavian-negru:provide-default-for-application-field-2

Conversation

@octavian-negru
Copy link
Contributor

@octavian-negru octavian-negru added WIP db-schema-changes This PR will require a Database Schema change labels Mar 31, 2020
@octavian-negru octavian-negru changed the title Provide default for application field 2 Bug 1625451 - Provide default for application field Mar 31, 2020
@octavian-negru octavian-negru removed the WIP label Apr 1, 2020
@octavian-negru
Copy link
Contributor Author

Local tests are passing, waiting for Travis to do its job...

Copy link
Contributor

@ionutgoldan ionutgoldan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than that, I think this looks ok.

@ionutgoldan
Copy link
Contributor

@octavian-negru could you rebase on lastest master?

@codecov-io
Copy link

codecov-io commented Apr 2, 2020

Codecov Report

Merging #6213 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6213   +/-   ##
=======================================
  Coverage   75.43%   75.44%           
=======================================
  Files         464      465    +1     
  Lines       19233    19237    +4     
  Branches     1531     1531           
=======================================
+ Hits        14509    14513    +4     
  Misses       4409     4409           
  Partials      315      315           
Impacted Files Coverage Δ
...igrations/0028_default_application_to_empty_str.py 100.00% <100.00%> (ø)
treeherder/perf/models.py 94.17% <100.00%> (ø)

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 f5b0a19...232a232. Read the comment docs.

@ionutgoldan
Copy link
Contributor

The equivalent raw SQL for the migration script looks like:

--
-- Alter field application on performancesignature
--
ALTER TABLE `performance_signature` ALTER COLUMN `application` SET DEFAULT '';
UPDATE `performance_signature` SET `application` = '' WHERE `application` IS NULL;
ALTER TABLE `performance_signature` MODIFY `application` varchar(10) NOT NULL;
ALTER TABLE `performance_signature` ALTER COLUMN `application` DROP DEFAULT;

@ionutgoldan ionutgoldan self-requested a review April 2, 2020 12:57
@ionutgoldan
Copy link
Contributor

Hi @sarah-clements! Could you take a look over this PR?
Thanks in advance!

@ionutgoldan ionutgoldan removed the request for review from airimovici April 2, 2020 12:59
Copy link
Contributor

@sarah-clements sarah-clements left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ionutgoldan ionutgoldan merged commit c4f48d8 into mozilla:master Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

db-schema-changes This PR will require a Database Schema change python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants