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

Don't depend on tempfile #12

Closed
jacobsvante opened this issue Nov 7, 2022 · 6 comments
Closed

Don't depend on tempfile #12

jacobsvante opened this issue Nov 7, 2022 · 6 comments
Assignees
Labels
enhancement New feature or request in progress Currently under investigation or development

Comments

@jacobsvante
Copy link

Feature Request

As can be seen here, rust_xlsxwriter currently depends on the tempfile crate, which in turn means that a /tmp directory (or equivalent) needs to be available. This isn't the case in Kubernetes environments by default, which is how I noticed this. I guess wasm environments would also have trouble with this.

Wouldn't it make more sense to default to in-memory storage, and make tempfile an optional feature to the crate? Then with that feature enabled, one could opt in to the file-creating behavior using an associated function, e.g. Workbook::new_tempfile_backed().

@jacobsvante
Copy link
Author

jacobsvante commented Nov 7, 2022

Another API suggestion:

let mut wb = Workbook::new();
wb.use_tempfile_when_larger_than(5_000_000);

In this case temp file would never be created unless data grows to a size of 5 MBytes or more.

@jmcnamara
Copy link
Owner

jmcnamara commented Nov 7, 2022

Thanks for the feedback.

rust_xlsxwriter currently depends on the tempfile crate, which in turn means that a /tmp directory (or equivalent) needs to be available.

There are two items on the roadmap to deal with that:

  • In memory mode. Which avoids tempfiles for systems that don't have them, and
  • Tempfile directory support. Which allows you to specify a location other than "/tmp" as the temp directory.

The Python version supports both of those: https://xlsxwriter.readthedocs.io/workbook.html#constructor

However, I will need to balance this with a 'low/constant memory mode' feature (also supported in the Python version) where tempfile will be required as a backend for worksheets.

However, in the short term I may just remove the tempfile requirement (if there is no performance difference) and re-introduce it later as an optional feature for constant memory mode.

jmcnamara added a commit that referenced this issue Nov 8, 2022
The xmlwriter was structured to use tempfiles for storing
xml data prior to writing to the combined zip file. This
has been replaced with in-memory cursors.

Hopefully this will help portability to OSes with limited
or no access to /tmp.

Feature request #12
@jmcnamara
Copy link
Owner

I've pushed a version to main that removes the dependency on the tempfile crate. You can test it if you get a chance.

@jmcnamara jmcnamara self-assigned this Nov 8, 2022
@jmcnamara jmcnamara added enhancement New feature or request in progress Currently under investigation or development labels Nov 8, 2022
@jmcnamara
Copy link
Owner

Thanks for the suggestion.

Fixed in version 0.12.1. Closing.

@jacobsvante
Copy link
Author

Great job! Thank you!!

@jacobsvante
Copy link
Author

However, I will need to balance this with a 'low/constant memory mode' feature (also supported in the Python version) where tempfile will be required as a backend for worksheets.

Agreed. Having this option will most likely be important for many scenarios, no matter how memory efficient Rust is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request in progress Currently under investigation or development
Projects
None yet
Development

No branches or pull requests

2 participants