abhinav-phi commented on PR #2122: URL: https://github.com/apache/stormcrawler/pull/2122#issuecomment-5600975024
Agreed on the design — pushed 97e30bc5. The store-side normalisation now lives in `AbstractStatusUpdaterBolt.execute`, exactly where you and @jnioche pointed: - only when the status is `DISCOVERED` (the existing `potentiallyNew` flag) - before the dedup-cache lookup at :157 and before the `AS_IS_NEXTFETCHDATE` early return, so the cache and the scheduler see the same key the record is stored under - behind `status.updater.normalise.hosts`, **default false**, with your `getDocumentID` point written into the key's javadoc and the yaml comment as the release note On shrinking the PR: I kept the read-side canonicalisation on purpose. While the flag is off (the default, for a whole release) and for URLs stored before normalisation existed, the store still hands raw-alias URLs back to the spouts, so the politeness-queue id and the robots-cache key would split them again — exactly the `byIP`/`byDomain`-needs-a-canonical-host case you flagged. It's the same `getCanonicalHost`, so no new concept. If you'd still rather ship only the store-side change now and keep the read path for the flag-on flip, say so and I'll drop the fetcher/partitioner hunks — easy to split. `normaliseHost` only rewrites the authority's host and refuses to touch anything that could corrupt it (IPv6 literals, hostless/non-hierarchical URLs, or a decoded escape that would yield `:` `/` `?` `#` `@`), and it's idempotent — covered by `URLUtilNormaliseHostTest`. The storage/dedup/default-off/only-DISCOVERED behaviours are pinned in `AbstractStatusUpdaterBoltTest`. Full core suite green (449). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
