Skip to content

Conversation

@bernhardkaindl
Copy link
Contributor

@bernhardkaindl bernhardkaindl commented Jun 9, 2023

The commits on this PR depend on the 1st commit:

It adds Accessor.openText() as a new API to make uses like ConfigParser().read_file(TextIO) which need TextIO(IO[str]). It replaces calls like these (change commit: a97be27?diff=split)

           rawtreeinfofp = access.openAddress(cls.TREEINFO_FILENAME)
            if sys.version_info < (3, 0) or isinstance(rawtreeinfofp, io.TextIOBase):
                # e.g. with FileAccessor
                treeinfofp = rawtreeinfofp
            else:
                # e.g. with HTTPAccessor
                treeinfofp = io.TextIOWrapper(rawtreeinfofp, encoding='utf-8')
            treeinfo = configparser.ConfigParser()
            treeinfo.read_file(treeinfofp)
            if treeinfofp != rawtreeinfofp:
                treeinfofp.close()
            rawtreeinfofp.close()

with:

            with access.openText(cls.TREEINFO_FILENAME) as fp:
                treeinfo.read_file(fp)

It makes it easy to use TextIO(IO[str]) with xcp.accessor on Python3 by using a context manager which takes care of closing all buffers/readers after leaving the context:

@codecov
Copy link

codecov bot commented Jun 9, 2023

Codecov Report

Merging #76 (eb14ba0) into master (6e930d1) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #76      +/-   ##
==========================================
+ Coverage   77.59%   77.62%   +0.03%     
==========================================
  Files          21       21              
  Lines        3468     3473       +5     
==========================================
+ Hits         2691     2696       +5     
  Misses        777      777              
Flag Coverage Δ
unittest 77.62% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
xcp/accessor.py 86.83% <100.00%> (+0.63%) ⬆️
xcp/repository.py 74.44% <100.00%> (-0.87%) ⬇️

@bernhardkaindl bernhardkaindl changed the title Accessor open text contextmanager Add Accessor.openText() as context manager for openAddress() Jun 9, 2023
Add `Accessor.openText()` as context manager to wrap openAddress() with
`io.TextIOWrapper` (if needed). It yields IO[str] or False, closing the
created TextIOWrapper and the underlying file on leaving the context.

It is intented for use in xcp.repository and any other user requiring
text as string without having to call .decode() on the returned file
object. It also closes the underlying file object on leaving the context.

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>
Use Accessor.openText() in _getVersion() for ConfigParser().read_file().
It simplifies _getVersion() dramatically and causes Accessor.openText()
to be tested with xcp.accessor.FileAccessor and xcp.accessor.HTTPAccessor.

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>
Test openText() with subclasses of xcp.accessor.MountingAccessor

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>
New test: Download a text file containing UTF-8 using
FTPAccessor.openText() and compare the returned string.

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>
Add test to get text containing UTF-8 using HTTPAccessor.openText()
and compare the returned decoded string contents.

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>
Split `assert_http_get_request_data()` using contextlib.contextmanager

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>
@bernhardkaindl bernhardkaindl force-pushed the accessor-openText-contextmanager branch from 448de5f to eb14ba0 Compare June 9, 2023 23:42
@bernhardkaindl bernhardkaindl merged commit 0d3d93b into xenserver:master Jun 13, 2023
@bernhardkaindl bernhardkaindl deleted the accessor-openText-contextmanager branch September 26, 2025 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants