[MISC] Fix Azure listings_expiry_time regression from PR #1826#1839
Merged
gaya3-zipstack merged 1 commit intomainfrom Mar 10, 2026
Merged
[MISC] Fix Azure listings_expiry_time regression from PR #1826#1839gaya3-zipstack merged 1 commit intomainfrom
gaya3-zipstack merged 1 commit intomainfrom
Conversation
Revert listings_expiry_time back to 1 (from 0). A value of 0 breaks because
fsspec's DirCache expires entries between write and immediate read, causing
KeyError('/') in adlfs's _ls_containers(). Updated comment to document the
issue and reference the related GitHub issue.
Contributor
Test ResultsSummary
Runner Tests - Full Report
|
Contributor
Summary by CodeRabbitRelease Notes
WalkthroughThe change adjusts the minimum Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
chandrasekharan-zipstack
requested review from
gaya3-zipstack,
kirtimanmishrazipstack and
muhammad-ali-e
chandrasekharan-zipstack
changed the title
muhammad-ali-e
approved these changes
Mar 10, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
unstract/connectors/src/unstract/connectors/filesystems/azure_cloud_storage/azure_cloud_storage.py (1)
42-51: Consider making the cache requirement explicit for resilience.The regression fix correctly pins
listings_expiry_time=1, which resolves theKeyError('/')caused by an expiry time of 0. However, the code omitsuse_listings_cacheand relies on the fsspec default. Since your comment explicitly documents thatuse_listings_cache=Falsewould trigger the same failure, consider addinguse_listings_cache=Trueexplicitly to decouple correctness from library defaults.♻️ Suggested enhancement
self.azure_fs = AzureBlobFileSystem( account_name=account_name, credential=access_key, + use_listings_cache=True, listings_expiry_time=1, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@unstract/connectors/src/unstract/connectors/filesystems/azure_cloud_storage/azure_cloud_storage.py` around lines 42 - 51, The AzureBlobFileSystem instantiation currently sets listings_expiry_time=1 to avoid a KeyError but leaves use_listings_cache to the fsspec default; explicitly pass use_listings_cache=True when creating AzureBlobFileSystem (the azure_fs assignment / AzureBlobFileSystem(...) call) so the behavior is not dependent on upstream defaults and update the surrounding comment to note that use_listings_cache must be True for resilience.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@unstract/connectors/src/unstract/connectors/filesystems/azure_cloud_storage/azure_cloud_storage.py`:
- Around line 42-51: The AzureBlobFileSystem instantiation currently sets
listings_expiry_time=1 to avoid a KeyError but leaves use_listings_cache to the
fsspec default; explicitly pass use_listings_cache=True when creating
AzureBlobFileSystem (the azure_fs assignment / AzureBlobFileSystem(...) call) so
the behavior is not dependent on upstream defaults and update the surrounding
comment to note that use_listings_cache must be True for resilience.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b4c46465-12fe-4e25-a358-8c8a2cfe4ba9
📒 Files selected for processing (1)
unstract/connectors/src/unstract/connectors/filesystems/azure_cloud_storage/azure_cloud_storage.py
gaya3-zipstack
approved these changes
Mar 10, 2026
Contributor
gaya3-zipstack
left a comment
gaya3-zipstack
left a comment
There was a problem hiding this comment.
Looks ok for now. Approving with the understading that this may not fix all the problems.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Reverted
listings_expiry_timefrom 0 back to 1 in Azure Cloud Storage filesystem configuration. Updated the explanatory comment to document why 0 breaks and reference the related fsspec/adlfs issue.Why
PR #1826 changed the value to 0 based on a review comment, but this breaks Azure file listing operations. fsspec's DirCache expires entries between the write and immediate read operations, causing a KeyError('/') in adlfs's
_ls_containers()function. This surfaces as "Error listing files. '/'" to users.How
listings_expiry_timeparameter back to 1 (minimum safe value)AzureBlobFileSystem.lsimplementation is incompatible withuse_listings_cache=Falsefsspec/adlfs#230 for contextCan this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
No. This fix restores the working behavior prior to PR #1826. Azure Cloud Storage file listing will work correctly again.
Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
Dependencies Versions
Notes on Testing
Screenshots
None
Checklist
I have read and understood the Contribution Guidelines.