Skip to content

tmpfile does not end files with period on empty extension#9429

Merged
gjoseph92 merged 3 commits intodask:mainfrom
hendrikmakait:tmpfile-deal-with-empty-extension
Aug 25, 2022
Merged

tmpfile does not end files with period on empty extension#9429
gjoseph92 merged 3 commits intodask:mainfrom
hendrikmakait:tmpfile-deal-with-empty-extension

Conversation

@hendrikmakait
Copy link
Member

@hendrikmakait hendrikmakait commented Aug 25, 2022

As it turns out, when not specifying an extension, tmpfile ends the filename with a period, which is discouraged on Windows (https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file) and caused issues fixed in dask/distributed#6954 as other system utilities strip the period away.

  • Tests added / passed
  • Passes pre-commit run --all-files

Copy link
Collaborator

@gjoseph92 gjoseph92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this here too!

@gjoseph92
Copy link
Collaborator

I'm not familiar enough with dask's CI to know if these failures are flaky, or if this actually broke something?

  • dask/bytes/tests/test_http.py::test_parquet[fastparquet] (this is failing everywhere so it's probably actually broken)
  • dask/tests/test_distributed.py::test_blockwise_dataframe_io[True-True-hdf]
  • dask/tests/test_distributed.py::test_blockwise_dataframe_io[True-None-hdf]

@ian-r-rose
Copy link
Collaborator

dask/tests/test_distributed.py::test_blockwise_dataframe_io[True-True-hdf]

This looks like #8816, a known flaky test related to HDF5 and distributed disagreeing about whether something is locked.

@hendrikmakait
Copy link
Member Author

dask/bytes/tests/test_http.py::test_parquet[fastparquet] looks like #9424

@jrbourbeau
Copy link
Member

Just merged main which should fix the test_parquet failure

@gjoseph92 gjoseph92 merged commit bf66221 into dask:main Aug 25, 2022
@gjoseph92
Copy link
Collaborator

Thanks @hendrikmakait @jrbourbeau!

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.

4 participants