Revert "Restore 'offset' and 'negative_utc' arguments and make them optional"
ClosedPublic
Actions

Authored by vlorentz on Jan 21 2022, 2:54 PM.

Details

Reviewers

ardumont

Group Reviewers

Reviewers

Maniphest Tasks

T3752: Store/represent time offsets as strings

Commits

rDMOD07c7b4f15102: Revert "Restore 'offset' and 'negative_utc' arguments and make them optional"

Summary

This reverts commit a0f5436273ef6f5b62a388ae131ed5afa7287d00
(ie. the second half of D6936, that was added after review).

This means this commit removes the 'offset' and 'negative_utc'
arguments.

It also removes the 'negative_utc' attribute (which is not used
anymore), but keeps an 'offset' property, which is an alias to
'offset_minutes()'. This is only to keep this commit readable;
the next commit will remove this alias.

This also affects the journal, as these fields are removed from .to_dict() -> D7003

Diff Detail

Repository

rDMOD Data model

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.Jan 21 2022, 2:54 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptJan 21 2022, 2:54 PM

Build is green

Patch application report for D7011 (id=25415)

Rebasing onto a654060bf2...

Current branch diff-target is up to date.

Changes applied before test

commit e3b5349bbd685d364a2f62f627ff53b5fb9a943b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Jan 21 14:51:18 2022 +0100

    Revert "Restore 'offset' and 'negative_utc' arguments and make them optional"
    
    This reverts commit a0f5436273ef6f5b62a388ae131ed5afa7287d00.
    
    This means this commit removes the 'offset' and 'negative_utc'
    arguments.
    
    It also removes the 'negative_utc' attribute (which is not used
    anymore), but keeps an 'offset' property, which is an alias to
    'offset_minutes()'. This is only to keep this commit readable;
    the next commit will remove this alias.

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

Harbormaster completed remote builds in B26258: Diff 25415.Jan 21 2022, 2:56 PM

vlorentz requested review of this revision.Jan 21 2022, 2:56 PM

vlorentz added a child revision: D7012: Remove deprecated property 'TimestampWithTimezone.offset'.Jan 21 2022, 2:58 PM

vlorentz mentioned this in D7003: journal: Document the new format for gitdate..Jan 21 2022, 3:00 PM

vlorentz edited the summary of this revision. (Show Details)Jan 21 2022, 3:05 PM

lgtm

couple of question/suggestion inline.

swh/model/model.py
554	add logger.warn to check if that happens often or not (assuming we'll look at logs)?
swh/model/tests/test_identifiers.py
1053	--700?

This revision is now accepted and ready to land.Jan 26 2022, 11:34 AM

vlorentz added inline comments.Jan 26 2022, 11:37 AM

swh/model/model.py
554	even if we do see it in the logs, there is nothing we can do about it.
swh/model/tests/test_identifiers.py
1053	Yes. I kid you not, there are a few hundred commits with that specific timezone: T75#71479 Dulwich had issues with it as well. https://bugs.launchpad.net/dulwich/+bug/697828

rebase

Build is green

Patch application report for D7011 (id=26582)

Rebasing onto 118c8a44d7...

Current branch diff-target is up to date.

Changes applied before test

commit 07c7b4f151022f8ecdbaff6013cc71e36340d155
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Jan 21 14:51:18 2022 +0100

    Revert "Restore 'offset' and 'negative_utc' arguments and make them optional"
    
    This reverts commit a0f5436273ef6f5b62a388ae131ed5afa7287d00.
    
    This means this commit removes the 'offset' and 'negative_utc'
    arguments.
    
    It also removes the 'negative_utc' attribute (which is not used
    anymore), but keeps an 'offset' property, which is an alias to
    'offset_minutes()'. This is only to keep this commit readable;
    the next commit will remove this alias.

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

Harbormaster completed remote builds in B27462: Diff 26582.Mar 16 2022, 10:32 AM

Closed by commit rDMOD07c7b4f15102: Revert "Restore 'offset' and 'negative_utc' arguments and make them optional" (authored by vlorentz). · Explain WhyMar 16 2022, 10:33 AM

This revision was automatically updated to reflect the committed changes.

vlorentz added a commit: rDMOD07c7b4f15102: Revert "Restore 'offset' and 'negative_utc' arguments and make them optional".

vlorentz added a task: T3752: Store/represent time offsets as strings.Mar 16 2022, 10:36 AM