Skip to content
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

Only close our own opened files at go() #104

Closed
wants to merge 2 commits into from

Conversation

rapto
Copy link
Contributor

@rapto rapto commented Dec 17, 2022

We should only close files we opened. The practical problem solved is using a Django response or IO as output file. I have added check for closing the xml file, as I suppose the problem may arise when using a stream as xml input file.

@regebro
Copy link
Contributor

regebro commented Feb 24, 2023

You are quite correct, but I implemented this with context managers.

Thanks!

@regebro regebro closed this Feb 24, 2023
@rapto rapto deleted the zope_master branch February 24, 2023 19:58
@orainova-ingentics
Copy link

orainova-ingentics commented Apr 25, 2023

It seems like this issue is still present? @regebro could you please elaborate on your workaround to avoid closing files? Currently, we're unable to use the library unless I fork it to fix closing the external BytesIO. Shall I open a new issue, since @rapto's PR has not been merged?

Edit: while I'm still curious why it's not been fixed yet, and about regebro's own workaround, I'll provide my own wrapper, in case someone comes across this issue in the future and needs it working:
`

with BytesIO(dataRml) as inputRml:
... 
  with BytesIO() as resultPdf:
    self.rml2pdf(inputRml, resultPdf)
...
def rml2pdf(self, rml, pdf):

    root = etree.parse(rml).getroot()
    doc = document.Document(root)
    doc.filename = rml

    try:
        doc.process(pdf)
    except Exception as err:
        print("Pdf generator error: %s" % err)
        traceback.print_exc()

`

@rapto
Copy link
Contributor Author

rapto commented Apr 25, 2023

This (as current in master):

    if hasattr(xmlInputName, 'read'):
        # it is already a file-like object
        xmlFile = xmlInputName
        xmlInputName = 'input.pdf'
    else:
        with open(xmlInputName, 'rb') as xmlFile:
            return go(xmlFile, outputFileName, outDir, dtdDir)

works fine in my case

@regebro
Copy link
Contributor

regebro commented Apr 25, 2023

@orainova-ingentics A version with the changes has not been released, and it should be, that's correct.

My fix looks at the input, and if it's an open file, it does not open or close it. If it is a path it will open it with a context manager. That ensures that it is closed.

@regebro
Copy link
Contributor

regebro commented Apr 25, 2023

Version 4.3.0 has now been released.

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.

None yet

3 participants