Skip to content

fix(windows): replace NamedTemporaryFile with TemporaryFile#74

Merged
FlipperPA merged 1 commit intodjango-commons:mainfrom
rdmolony:replace-namedtemporaryfile-with-temporaryfile
Jun 23, 2023
Merged

fix(windows): replace NamedTemporaryFile with TemporaryFile#74
FlipperPA merged 1 commit intodjango-commons:mainfrom
rdmolony:replace-namedtemporaryfile-with-temporaryfile

Conversation

@rdmolony
Copy link
Copy Markdown
Contributor

@rdmolony rdmolony commented Mar 6, 2023

Hi there,

Thanks for creating & maintaining drf-excel 😃

I'm running drf on Windows Server 2019 (Datacenter) for legacy/historical reasons. This version blocks NamedTemporaryFile from saving to temp dir.

https://stackoverflow.com/questions/23212435/permission-denied-to-write-to-my-temporary-file

TemporaryFile seems to work fine across platforms in its place as it is not blocked by the dreaded permissions

Windows blocked NamedTemporaryFile from saving to temp dir
... using TemporaryFile instead works fine
@FlipperPA
Copy link
Copy Markdown
Member

Huh, it is odd that NamedTemporaryFile got used without using an explicit name. Let me take a look a bit more, but in concept I'll get this merged in a day or two.

@AlexKovyazin
Copy link
Copy Markdown

AlexKovyazin commented Jun 13, 2023

Got this problem too.
https://stackoverflow.com/questions/66744497/python-tempfile-namedtemporaryfile-cant-use-generated-tempfile

The following adjustments also solved the problem:

def _save_virtual_workbook(self, wb):
    try:
        tmp = NamedTemporaryFile(delete=False)
        save_workbook(wb, tmp.name)
        tmp.seek(0)
        virtual_workbook = tmp.read()
    finally:
        tmp.close()
        os.unlink(tmp.name)

    return virtual_workbook

@FlipperPA
Copy link
Copy Markdown
Member

Sorry for the delay, just back from vacation. I'll get a version published to PyPI today. @rdmolony @AlexKovyazin

@FlipperPA FlipperPA merged commit 83e5100 into django-commons:main Jun 23, 2023
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