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

Fix URI cache to cache URIs across multiple files #348

Closed
MichaIng opened this issue Sep 30, 2021 · 4 comments · Fixed by #443
Closed

Fix URI cache to cache URIs across multiple files #348

MichaIng opened this issue Sep 30, 2021 · 4 comments · Fixed by #443

Comments

@MichaIng
Copy link
Member

As tested here, the URI cache for caching URI check results across all input files, does not work as expected.

There is a test for repetition in a single file: https://github.com/lycheeverse/lychee/blob/master/lychee-bin/tests/cli.rs#L218-L229
But this worked before the new URI cache was implemented. This test may be extended to use two identical files.
@mre
Is it actually intended that two identical URLs in two separate files contribute only 1 to the total of the summary, or is it still 1 per input (and only the actual request skipped internally)? In the latter case, expanding or implementing the test would be more difficult.

@mre
Copy link
Member

mre commented Sep 30, 2021

The cache should work across files. At least that's the goal. So the same link should always only count as 1 in the total, no matter in how many files it occurs.

@MichaIng
Copy link
Member Author

Okay great, I'll try to extend the cache accordingly.

@MichaIng
Copy link
Member Author

PR up: #349
I hope I did the syntax right 😅.

@MichaIng MichaIng changed the title Fix URI cache do cache URIs across multiple files Fix URI cache to cache URIs across multiple files Sep 30, 2021
@mre
Copy link
Member

mre commented Nov 22, 2021

Current status: Caching across files is currently disabled. See test_caching_across_files.
We're planning to bring this back at some point.
#349 (comment)

@mre mre mentioned this issue Jan 4, 2022
4 tasks
@mre mre closed this as completed in #443 Jan 14, 2022
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 a pull request may close this issue.

2 participants