Page MenuHomeSoftware Heritage
Differential D7354

backfill: Make integer_ranges() work on str args + add typing to RANGE_GENERATORS
ClosedPublic
Actions

Authored by vlorentz on Mar 15 2022, 5:17 PM.

Details

Reviewers
olasd
Group Reviewers
Reviewers
Commits
rDSTO8b65e42f5adb: backfill: Make integer_ranges() work on str args + add typing to…
Summary

Without the type annotation, mypy errors with 'Cannot call function of unknown type'
when called from a type-checked function.

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.Mar 15 2022, 5:17 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptMar 15 2022, 5:17 PM
vlorentz mentioned this in D7352: backfill: Add typing to RANGE_GENERATORS..Mar 15 2022, 5:18 PM
vlorentz updated this revision to Diff 26575.Mar 15 2022, 5:18 PM

reduce diff + add comment

Harbormaster returned this revision to the author for changes because remote builds failed.Mar 15 2022, 5:19 PM
Harbormaster failed remote builds in B27455: Diff 26575!
swh-public-ci added a comment.Mar 15 2022, 5:19 PM

Build was aborted

Patch application report for D7354 (id=26574)

Rebasing onto 77f7e6d10a...

Current branch diff-target is up to date.
Changes applied before test
commit 7c45c715094ecf37fde00795bd1990df5c673441
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 15 16:56:40 2022 +0100

    backfill: Make integer_ranges() work on str args + add typing to RANGE_GENERATORS
    
    Without the type annotation, mypy errors with 'Cannot call function of unknown type'
    when called from a type-checked function.

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

Harbormaster failed remote builds in B27454: Diff 26574!Mar 15 2022, 5:19 PM
swh-public-ci added a comment.Mar 15 2022, 5:19 PM

Build was aborted

Patch application report for D7354 (id=26575)

Rebasing onto 77f7e6d10a...

Current branch diff-target is up to date.
Changes applied before test
commit de44c76abb0028adc630b67b2aee026cd5b34fbc
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 15 16:56:40 2022 +0100

    backfill: Make integer_ranges() work on str args + add typing to RANGE_GENERATORS
    
    Without the type annotation, mypy errors with 'Cannot call function of unknown type'
    when called from a type-checked function.

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

vlorentz updated this revision to Diff 26576.Mar 15 2022, 5:20 PM

remove incorrect comment

swh-public-ci added a comment.Mar 15 2022, 5:29 PM

Build is green

Patch application report for D7354 (id=26576)

Rebasing onto 77f7e6d10a...

Current branch diff-target is up to date.
Changes applied before test
commit 8b65e42f5adbeef9fd1ef8e28a908c82ee77204f
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Mar 15 16:56:40 2022 +0100

    backfill: Make integer_ranges() work on str args + add typing to RANGE_GENERATORS
    
    Without the type annotation, mypy errors with 'Cannot call function of unknown type'
    when called from a type-checked function.

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

Harbormaster completed remote builds in B27456: Diff 26576.Mar 15 2022, 5:29 PM
vlorentz requested review of this revision.Mar 15 2022, 5:29 PM
olasd accepted this revision.Mar 16 2022, 3:55 PM
olasd added a subscriber: olasd.

Cool, thanks.

This revision is now accepted and ready to land.Mar 16 2022, 3:55 PM
Closed by commit rDSTO8b65e42f5adb: backfill: Make integer_ranges() work on str args + add typing to… (authored by vlorentz). · Explain WhyMar 16 2022, 4:09 PM
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDSTO8b65e42f5adb: backfill: Make integer_ranges() work on str args + add typing to….

Revision Contents
Changeset List

PathSize
swh/
storage/
49 lines

Diff 26616

View Options

swh/storage/backfill.py

Show First 20 Lines • Show All 51 Lines • ▼ Show 20 Linesdef _write_obj_file(hex_obj_id, objstorage):
(tmp, tmp_path) = tempfile.mkstemp(suffix='.tmp', prefix='hex_obj_id.', (tmp, tmp_path) = tempfile.mkstemp(suffix='.tmp', prefix='hex_obj_id.',
dir=dir) dir=dir)
# Open the file and yield it for writing. # Open the file and yield it for writing.
tmp_f = os.fdopen(tmp, 'wb') tmp_f = os.fdopen(tmp, 'wb')
with gzip.GzipFile(filename=tmp_path, fileobj=tmp_f) as f: with gzip.GzipFile(filename=tmp_path, fileobj=tmp_f) as f:
yield f yield f
# Make sure the contents of the temporary file are written to disk
tmp_f.flush()
if objstorage.use_fdatasync:
os.fdatasync(tmp)
else:
os.fsync(tmp)
# Then close the temporary file and move it to the right directory. # Then close the temporary file and move it to the right directory.
tmp_f.close() tmp_f.close()
os.chmod(tmp_path, FILE_MODE) os.chmod(tmp_path, FILE_MODE)
os.rename(tmp_path, path) os.rename(tmp_path, path)
@contextmanager @contextmanager
def _read_obj_file(hex_obj_id, objstorage): def _read_obj_file(hex_obj_id, objstorage):
▲ Show 20 Lines • Show All 52 Lines • ▼ Show 20 Lines def __init__(self, root, slicing, **kwargs):
self.root = root self.root = root
# Make a list of tuples where each tuple contains the beginning # Make a list of tuples where each tuple contains the beginning
# and the end of each slicing. # and the end of each slicing.
self.bounds = [ self.bounds = [
slice(*map(int, sbounds.split(':'))) slice(*map(int, sbounds.split(':')))
for sbounds in slicing.split('/') for sbounds in slicing.split('/')
if sbounds if sbounds
] ]
zackUnsubmitted
Not Done
Inline Actions

Is this because some python versions don't have it, or just for OS portability?

I'm asking because we now have evidence (in T1817) that fdatasync is critical for us, in the sense that we can lose data if it's not available. So just silently ignoring the fact it's not available makes me shiver :-)

I'm tempted to suggest failing to initialize if fdatasync is not available.

Either way, we should log an error message if it's not available.

zack: Is this because some python versions don't have it, or just for OS portability? I'm asking…
olasdAuthorUnsubmitted
Done
Inline Actions

The new version falls back to fsync (available on all platforms) if fdatasync isn't available.

olasd: The new version falls back to fsync (available on all platforms) if fdatasync isn't available.
zackUnsubmitted
Not Done
Inline Actions

*thumb up*

zack: *thumb up*
self.use_fdatasync = hasattr(os, 'fdatasync')
self.check_config(check_write=False) self.check_config(check_write=False)
def check_config(self, *, check_write): def check_config(self, *, check_write):
"""Check whether this object storage is properly configured""" """Check whether this object storage is properly configured"""
root = self.root root = self.root
if not os.path.isdir(root): if not os.path.isdir(root):
▲ Show 20 Lines • Show All 238 Lines • Show Last 20 Lines
About · Developer information · Legal