Skip to content

Add Accessor.openText() as context manager for openAddress() #76

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

bernhardkaindl
Copy link
Collaborator

@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 <[email protected]>
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 <[email protected]>
Test openText() with subclasses of xcp.accessor.MountingAccessor

Signed-off-by: Bernhard Kaindl <[email protected]>
New test: Download a text file containing UTF-8 using
FTPAccessor.openText() and compare the returned string.

Signed-off-by: Bernhard Kaindl <[email protected]>
Add test to get text containing UTF-8 using HTTPAccessor.openText()
and compare the returned decoded string contents.

Signed-off-by: Bernhard Kaindl <[email protected]>
Split `assert_http_get_request_data()` using contextlib.contextmanager

Signed-off-by: Bernhard Kaindl <[email protected]>
@bernhardkaindl bernhardkaindl force-pushed the accessor-openText-contextmanager branch from 448de5f to eb14ba0 Compare June 9, 2023 23:42
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