Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Reimplimented recursive downloads #377

Merged

Conversation

joereuss12
Copy link
Contributor

The functionality of recursive downloads has been restored. When specifying with the -r flag and targetting a dir, all the files will be downloaded within that directory. This functionality currently works however the progress bars are a little funky so that still needs fixing.

@joereuss12 joereuss12 linked an issue Nov 15, 2023 that may be closed by this pull request
@joereuss12
Copy link
Contributor Author

joereuss12 commented Nov 21, 2023

From playing around with things, it seems like the progress bars mostly look okay when the debug log is not activated. When debug is activated, it keeps wanting to print things during the download causing multiple bars for 1 file to be printed. I will create a new issue for this but I think we should implement what we have since it does work correctly. It also seems like it cannot get the correct total for the files when doing this recursively either.

Update: made issue #402 for this

@joereuss12 joereuss12 marked this pull request as ready for review November 21, 2023 21:43
Copy link
Collaborator

@bbockelm bbockelm left a comment

Choose a reason for hiding this comment

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

A small number of minor items.

Please also add a unit test covering the bearer token authentication before we do the final merge.

client/bearer_auth.go Show resolved Hide resolved
client/bearer_auth.go Outdated Show resolved Hide resolved
client/bearer_auth.go Outdated Show resolved Hide resolved
client/bearer_auth.go Outdated Show resolved Hide resolved
client/bearer_auth.go Outdated Show resolved Hide resolved
client/bearer_auth.go Outdated Show resolved Hide resolved
client/bearer_auth.go Outdated Show resolved Hide resolved
director/redirect.go Outdated Show resolved Hide resolved
joereuss12 and others added 4 commits November 27, 2023 16:44
The functionality of recursive downloads has been restored. When
specifying with the -r flag and targetting a dir, all the files will be
downloaded within that directory. This functionality currently works
however the progress bars are a little funky so that still needs fixing.
Made some updates to bearer_auth.go such as adding copywrite statement,
removing old commented out code, and changing types to not be exported.
Also changed `dirlisthost` to `collections-url` in the header.
@joereuss12 joereuss12 force-pushed the recursive-downloads-revisit-branch branch from 909a26b to b488c7d Compare November 27, 2023 20:21
@joereuss12 joereuss12 requested a review from bbockelm November 27, 2023 21:20
@joereuss12 joereuss12 marked this pull request as draft November 28, 2023 21:14
@joereuss12
Copy link
Contributor Author

Changed to be a draft because now I can make unit tests for this functionality from the merging of #390

@joereuss12
Copy link
Contributor Author

Marking this as ready for review since the end-to-end tests are currently stalled. See #391

Copy link
Collaborator

@bbockelm bbockelm left a comment

Choose a reason for hiding this comment

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

LGTM after the prior round of changes.

@bbockelm bbockelm merged commit ac1dbeb into PelicanPlatform:main Dec 1, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recursive downloads, revisited
2 participants