Differential D3805

pg: Check revision.extra_headers is not null.
ClosedPublic
Actions

Authored by vlorentz on Aug 19 2020, 2:26 PM.

Details

Reviewers

anlambert

Group Reviewers

Reviewers

Commits

rDSTO6a53cb3ef3ad: pg: Check revision.extra_headers is not null.

Summary

Fixes a regression in 038a219f84d6b8a4f02b48f9ad3c5d823d097790,
as it made the converter expect a list.

When adding this column, we made it default to null instead of defaulting
to an empty array, so existing records were initialized will null.

This commit migrates these nulls to empty arrays, then adds a constraint
to enforce it in the future.

Diff Detail

Repository

rDSTO Storage manager

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vlorentz created this revision.Aug 19 2020, 2:26 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptAug 19 2020, 2:26 PM

Build has FAILED

Patch application report for D3805 (id=13408)

Could not rebase; Attempt merge onto bd925475d9...

Updating bd925475..ee5ffc0a
Fast-forward
 sql/upgrades/160.sql               | 41 ++++++++++++++++++++++++++++++++++++++
 sql/upgrades/161.sql               | 17 ++++++++++++++++
 swh/storage/converters.py          | 16 +++++++++++----
 swh/storage/sql/30-swh-schema.sql  |  4 ++--
 swh/storage/sql/60-swh-indexes.sql | 22 ++++++++++++++++++++
 5 files changed, 94 insertions(+), 6 deletions(-)
 create mode 100644 sql/upgrades/160.sql
 create mode 100644 sql/upgrades/161.sql

Changes applied before test

commit ee5ffc0ade0fa2e54d62714ae825a8e76c7b53aa
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Aug 19 14:25:22 2020 +0200

    pg: Check revision.extra_headers is not null.
    
    Fixes a regression in 038a219f84d6b8a4f02b48f9ad3c5d823d097790,
    as it made the converter expect a list.
    
    When adding this column, we made it default to null instead of defaulting
    to an empty array, so existing records were initialized will null.
    
    This commit migrates these nulls to empty arrays, then adds a constraint
    to enforce it in the future.

commit 7e12798961b815fc20caa5f78a0e9a2428313ae5
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Aug 19 14:23:54 2020 +0200

    converters: convert extra_headers to an empty list if it is None.

commit 6b467dd4c12827a5d1ed347d3edcd079e4c03724
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Aug 19 14:06:21 2020 +0200

    converters: convert neg_utc_offset to False if it is None.

commit 131d156469724e07dd34efc5a47873c1b63097f5
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Aug 19 12:40:01 2020 +0200

    pg: Make date_neg_utc_offset is not null if date is not null.
    
    Fixes a regression in 038a219f84d6b8a4f02b48f9ad3c5d823d097790,
    as it made converters expect a boolean.
    
    We stopped writing this kind of nulls since we started using model
    objects for insertions, but didn't migrate existing data.
    
    This commit migrates these nulls to false, then adds a constraint
    to enforce it in the future.

Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/822/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/822/console

Harbormaster failed remote builds in B14672: Diff 13408!Aug 19 2020, 2:33 PM

rebase

Build is green

Patch application report for D3805 (id=13414)

Could not rebase; Attempt merge onto bd925475d9...

Updating bd925475..6a53cb3e
Fast-forward
 sql/upgrades/160.sql               | 41 ++++++++++++++++++++++++++++++++++++++
 sql/upgrades/161.sql               | 17 ++++++++++++++++
 swh/storage/backfill.py            |  1 +
 swh/storage/converters.py          | 16 +++++++++++----
 swh/storage/sql/30-swh-schema.sql  |  4 ++--
 swh/storage/sql/60-swh-indexes.sql | 22 ++++++++++++++++++++
 6 files changed, 95 insertions(+), 6 deletions(-)
 create mode 100644 sql/upgrades/160.sql
 create mode 100644 sql/upgrades/161.sql

Changes applied before test

commit 6a53cb3ef3ad769ab015fa011e0a14702d1a14cf
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Aug 19 14:25:22 2020 +0200

    pg: Check revision.extra_headers is not null.
    
    Fixes a regression in 038a219f84d6b8a4f02b48f9ad3c5d823d097790,
    as it made the converter expect a list.
    
    When adding this column, we made it default to null instead of defaulting
    to an empty array, so existing records were initialized will null.
    
    This commit migrates these nulls to empty arrays, then adds a constraint
    to enforce it in the future.

commit ca3ee92da47be37c25439a49ef18c4a1d63edd58
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Aug 19 14:23:54 2020 +0200

    converters: convert extra_headers to an empty list if it is None.

commit e2b14948da143925b0421415580df63e48d18a5b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Aug 19 12:40:01 2020 +0200

    pg: Make date_neg_utc_offset is not null if date is not null.
    
    Fixes a regression in 038a219f84d6b8a4f02b48f9ad3c5d823d097790,
    as it made converters expect a boolean.
    
    We stopped writing this kind of nulls since we started using model
    objects for insertions, but didn't migrate existing data.
    
    This commit migrates these nulls to false, then adds a constraint
    to enforce it in the future.

commit 7dcd5708b0cffdacd0082288262ccc4d78cec2de
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Aug 19 14:06:21 2020 +0200

    converters: convert neg_utc_offset to False if it is None.

commit e2f066549123f069bf0ae34d593038ffd834d9c4
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Aug 19 14:54:45 2020 +0200

    backfiller: Add missing 'extra_header' field.
    
    This field wasn't backfilled; and it wasn't caught by tests, because
    revisions in swh.journal.tests.journal_data.TEST_OBJECTS were had
    an empty extra_header field.
    
    Starting with swh-journal v0.4.3, one of the revisions in TEST_OBJECTS
    has a non-empty extra_header field, so it will become a test failure
    without this commit

See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/827/ for more details.

Harbormaster completed remote builds in B14678: Diff 13414.Aug 19 2020, 3:18 PM

anlambert accepted this revision.Aug 19 2020, 3:42 PM

This revision is now accepted and ready to land.Aug 19 2020, 3:42 PM

Closed by commit rDSTO6a53cb3ef3ad: pg: Check revision.extra_headers is not null. (authored by vlorentz). · Explain WhyAug 20 2020, 8:51 AM

This revision was automatically updated to reflect the committed changes.

vlorentz added a commit: rDSTO6a53cb3ef3ad: pg: Check revision.extra_headers is not null..