feat: Add waiting logic for FIFO files that may not be immediately readable #599
Conversation
Add support for reading .env files from FIFOs (named pipes) which may not have content immediately available when first accessed. This is useful for environments that expose .env data via FIFOs or mounted filesystems that may initially be empty. - Add _wait_for_file_content() function to handle both regular files and FIFOs with proper blocking/waiting logic - Update _get_stream() to use the new function for FIFO-aware file reading - Add select and time imports for FIFO handling
Add tests for _wait_for_file_content() to verify it correctly handles both regular files and FIFOs. The existing FIFO test worked because it started a writer thread before reading, ensuring the FIFO was ready. However, with 1Password's asynchronously mounted FIFO files, the FIFO may exist but not be immediately readable when first accessed. The old code would open FIFOs directly in text mode, which could block indefinitely waiting for a writer. The new _wait_for_file_content() function addresses this by: - Detecting FIFOs using stat.S_ISFIFO() - Using select.select() to wait for the FIFO to become readable - Reading content only when the FIFO is ready These tests verify: - Regular files continue to work correctly (no regression) - FIFOs are properly handled with waiting logic for async-mounted files
|
@sidharth-sudhir maybe you want to look at this too. Prior to this change, unless 1Password was already unlocked, it would not prompt a 1Password CTA |
Hey @lukeocodes, thanks for opening the issue and PR! Appreciate you taking the time to improve the FIFO support. I've been trying to reproduce the issue you described, but I'm having trouble recreating it with the current released version of python-dotenv. In my testing, even when 1Password is locked and I run an application that uses One thing I noticed: in the issue you wrote, you mention that you are on python-dotenv 1.0.1, but the basic FIFO support I added was only released in v1.2.1. Could you confirm which version you're testing with? If you're on 1.0.1, that would explain why you're seeing issues, since FIFO support wasn't available then. That said, the changes you've made in this PR look good and make sense to me. I'm just trying to understand the specific conditions where the current implementation fails so I can better validate the fix. Thanks again for the contribution, and I'm happy to help get this merged once we can align on reproducing the issue! |
|
It's really odd, I had 1Password working on python-dotenv 1.0.1 for month, today I had to bump to 1.1.1 and it stopped working. |
Does it work on v1.2.1? |
Unfortunately my deps require =1.1.1, I cannot bump it. |
I am working on our SDK here and if i try to run any of my examples using poetry then it immediately errors. Using my change it works great (using python-dotenv>=1.2.1 now) |
3 participants
Fixes #598
python-dotenvalready had basic FIFO support (added in #586), it didn't work with asynchronously mounted FIFO files, like by 1Password's new (beta) Environment featExisting implementation opened FIFOs directly:
The flaw with this approach: when a FIFO exists but isn't immediately ready to read (common with async-mounted filesystems),
open()can block indefinitely waiting for a writer, or read it as empty.The existing test (
test_load_dotenv_from_fifo) worked because it started a writer thread before attempting to read, ensuring the FIFO was ready and had contentSolution
I've added a new
_wait_for_file_content()function that handles FIFOs that may not be immediately readable:stat.S_ISFIFO()to distinguish them from regular filesselect.select()to wait for the FIFO to become readable (with a 5-second timeout)rb,buffering=0) for accurate readiness detectionStringIOobjectThe result is that even when using regular FIFO or using 1Password, it reads (or prompts 1Password for an unlock password) correctly
Changes
_wait_for_file_content()function insrc/dotenv/main.py_get_stream()to use the new function for FIFO-aware file readingTesting
Impact
This change is backward compatible and imprpves support for: