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

Ls revamp #87

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Ls revamp #87

wants to merge 4 commits into from

Conversation

turetske
Copy link
Collaborator

@turetske turetske commented Dec 12, 2024

A draft PR replacing _ls_real, it correctly works for find/walk which will correct glob's issues. It currently overwrites the httpfs version of _ls_real. This was done because otherwise _find, _walk, and _glob would also need to copied and added to pelicanfs which was giving me some bugs, but I can change it if you want it differently.

There are also TODOs in the comments for pieces that I am still working on.

Things that still need to be done/tested:

  • Unit tests: I want to write extensive unit tests for all these effected functions to ensure things still work
  • All the unit tests work locally, but the webdavhostname doesn't work for the unit tests. I'm sure it's just a matter of adding some sort of test server response to fix.
  • I need to test and ensure auth and token provisions in _ls_real and account for edge cases as well as ensure it gives what we expect when detail=False
  • Because httpfs's versions of find is calling our version of _ls_real, the full url names are being retained in the detailed returns. I need to do some more extensive end to end tests to ensure this doesn't break things. Especially with Harsha's intake catalogues, etc.

EDIT: Apologies for the many pushes, the rebase I did before this wasn't fully done properly. (So many linter fixes)

@turetske turetske requested a review from bbockelm December 12, 2024 22:34
@turetske turetske force-pushed the ls_revamp branch 2 times, most recently from 01fe85a to 38e38a4 Compare December 12, 2024 22:43
	-- This function will use a webdav listings for the base ls call
	-- This is to avoid having to create find and walk which would be simply be duplicates of the httpfs find/walk
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.

walk/glob/find don't recurse Expand Tests for 'find', 'glob', and 'walk'
1 participant