Improve UniqueRequest - Githubissues

elixir-crawly / crawly

Crawly, a high-level web crawling & scraping framework for Elixir.

https://hexdocs.pm/crawly

Apache License 2.0

988 stars 116 forks source link

Improve UniqueRequest #256

Closed serpent213 closed 1 year ago

serpent213 commented 1 year ago

optional hashing (closes elixir-crawly/crawly#143)
ignore trailing slash, assuming that pathes /a and /a/ yield identical content

oltarasenko commented 1 year ago

@serpent213 It looks good from my point of view. However, I don't see any reason why we want to make it optional. E.g. I don't see why this middleware should store the URL in any case if it's better to store just a hash. Could you please make a switch so it uses hash by default and urls are not used at all anymore?

serpent213 commented 1 year ago

@oltarasenko My reasoning was the following:

Correctness first – no need to even think about collisions when using the URL.
Memory – let's say we use SHA256 as default, that would mean 32 bytes. Some URLs are shorter than that, some are longer, depends on the use case. So I thought it should be a conscious choice; also the possibility to choose e.g. MD5 if memory is more important than correctness.
For most users it doesn't matter anyway, I imagine.

I could as well make it two different Middlewares, like UniqueRequest and HashedUniqueRequest.

oltarasenko commented 1 year ago

Hey, @serpent213 could you please re-base and format your code so I can merge it. Thanks for the contribution!

serpent213 commented 1 year ago

Hey, @serpent213 could you please re-base and format your code so I can merge it. Thanks for the contribution!

Looks good now? I hope, my git-fu didn't skip out on me. 😉