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.

Sign up for GitHub

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

Jump to bottom

implement subdomain focus feature in data-prep-connector #725

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
+58 −9
Open

implement subdomain focus feature in data-prep-connector #725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
477da0f
implement subdomain focus feature in data-prep-connector
hmtbr Oct 18, 2024
1722e36
refactoring
hmtbr Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions data-connector-lib/src/dpk_connector/core/crawler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ def async_crawl(
user_agent: str = "",
headers: dict[str, str] = {},
allow_domains: Collection[str] = (),
subdomain_focus: bool = False,
Copy link

Choose a reason for hiding this comment

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

Should the default be set to True? Its is very rare that we need to do a common crawl of the web and not be restricted/focused on a specific domain or a specific URL...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we set it true by default, the default behavior the crawler would change. We should not introduce any braking change except for major version updates.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok. that makes sense. I have the feeling that for most of the RAG use cases we will be setting this to true.

path_focus: bool = False,
allow_mime_types: Collection[str] = (
"application/pdf",
Expand All @@ -96,6 +97,7 @@ def async_crawl(
user_agent (str): The user agent string to use for the crawler. Defaults to "Scrapy/VERSION (+https://scrapy.org)".
headers (dict[str, str]): A dictionary of additional headers to send with each request. Default is an empty dictionary.
allow_domains (Collection[str]): A collection of domains to restrict the crawler to. Default is the domains of the seed URLs.
subdomain_focus (bool): If specified, only links under the subdomains of the input seed URLs will be extracted. Ignored if `allow_domains` is specified.
path_focus (bool): If specified, only links under the paths of the input seed URLs will be extracted.
allow_mime_types (Collection[str]): A collection of MIME types to allow during the crawl. Default is a collection containing "application/pdf", "text/html", "text/markdown", and "text/plain".
disallow_mime_types (Collection[str]): A collection of MIME types to disallow during the crawl. Default is an empty collection.
Expand Down Expand Up @@ -140,6 +142,7 @@ def async_crawl(
seed_urls=seed_urls,
callback=on_downloaded,
allow_domains=allow_domains,
subdomain_focus=subdomain_focus,
path_focus=path_focus,
allow_mime_types=allow_mime_types,
disallow_mime_types=disallow_mime_types,
Expand All @@ -155,6 +158,7 @@ def crawl(
user_agent: str = "",
headers: dict[str, str] = {},
allow_domains: Collection[str] = (),
subdomain_focus: bool = False,
path_focus: bool = False,
allow_mime_types: Collection[str] = (
"application/pdf",
Expand All @@ -177,6 +181,7 @@ def crawl(
user_agent (str): The user agent string to use for the crawler. Defaults to "Scrapy/VERSION (+https://scrapy.org)".
headers (dict[str, str]): A dictionary of additional headers to send with each request. Default is an empty dictionary.
allow_domains (Collection[str]): A collection of domains to restrict the crawler to. Default is the domains of the seed URLs.
subdomain_focus (bool): If specified, only links under the subdomains of the input seed URLs will be extracted. Ignored if `allow_domains` is specified.
path_focus (bool): If specified, only links under the paths of the input seed URLs will be extracted.
allow_mime_types (Collection[str]): A collection of MIME types to allow during the crawl. Default is a collection containing "application/pdf", "text/html", "text/markdown", and "text/plain".
disallow_mime_types (Collection[str]): A collection of MIME types to disallow during the crawl. Default is an empty collection.
Expand All @@ -198,6 +203,7 @@ def on_completed(result: Any):
user_agent,
headers,
allow_domains,
subdomain_focus,
path_focus,
allow_mime_types,
disallow_mime_types,
Expand Down
20 changes: 14 additions & 6 deletions data-connector-lib/src/dpk_connector/core/spiders/sitemap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
get_content_type,
get_etld1,
get_focus_path,
get_fqdn,
is_allowed_path,
urlparse_cached,
)
Expand All @@ -42,6 +43,7 @@ def __init__(
self,
seed_urls: Collection[str],
allow_domains: Collection[str] = (),
subdomain_focus: bool = False,
path_focus: bool = False,
allow_mime_types: Collection[str] = (),
disallow_mime_types: Collection[str] = (),
Expand Down Expand Up @@ -88,11 +90,15 @@ def __init__(
self.focus_paths.add(path)

# Domains and mime types filtering
self.allowed_domains = set(
allow_domains
if len(allow_domains) > 0
else [get_etld1(url) for url in seed_urls]
Copy link

Choose a reason for hiding this comment

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

Let's say the URL is : http://www.research.ibm.com and domain focus is set to true. Will this restrict to *.research.ibm.com or will it restrict to *.ibm.com ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note that the parameter I introduced this time is "sub"domain_focus. The crawler focuses the input domain by default: the seed url: https://www.research.ibm.com/ -> the crawl will be restricted to *.ibm.com
If subdomain_focus is set to true, the crawl will be restricted to *.www.research.ibm.com.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice! Thanks for the clarification @hmtbr -san

)
if allow_domains:
self.allowed_domains = set(allow_domains)
elif subdomain_focus:
self.allowed_domains = set()
for url in seed_urls:
if fqdn := get_fqdn(url):
self.allowed_domains.add(fqdn)
else:
self.allowed_domains = set(get_etld1(url) for url in seed_urls)
self.allow_mime_types = set(
[m.lower() for m in allow_mime_types] if len(allow_mime_types) > 0 else ()
)
Expand Down Expand Up @@ -155,7 +161,9 @@ def start_requests(self):
)

def _parse_sitemap(self, response: Response):
yield ConnectorItem(dropped=False, downloaded=False, system_request=True, sitemap=True)
yield ConnectorItem(
dropped=False, downloaded=False, system_request=True, sitemap=True
)

seed_url = response.meta["seed_url"]

Expand Down
5 changes: 5 additions & 0 deletions data-connector-lib/src/dpk_connector/core/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ def get_etld1(url: str) -> str:
return f"{ext.domain}.{ext.suffix}"


def get_fqdn(url: str) -> str:
ext = tldextract.extract(url)
return ext.fqdn


def get_focus_path(url: str) -> str | None:
parts = urlparse_cached(url)
if len(parts.path.split("/")) > 2:
Expand Down
20 changes: 17 additions & 3 deletions data-connector-lib/test/dpk_connector/core/test_sitemap_spider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
from pathlib import Path

import pytest
from dpk_connector.core.item import ConnectorItem
from dpk_connector.core.spiders.sitemap import BaseSitemapSpider, ConnectorSitemapSpider
from scrapy import Request
from scrapy.crawler import Crawler
from scrapy.http import HtmlResponse

from dpk_connector.core.item import ConnectorItem
from dpk_connector.core.spiders.sitemap import BaseSitemapSpider, ConnectorSitemapSpider


@pytest.fixture
def crawler() -> Crawler:
Expand All @@ -22,6 +21,21 @@ def crawler() -> Crawler:
return crawler


def test_init_subdomain_focus():
spider = BaseSitemapSpider(
seed_urls=(
"http://blog.example.com/",
"http://contents.example.com/",
),
subdomain_focus=True,
)
assert spider.seed_urls == {
"http://blog.example.com/",
"http://contents.example.com/",
}
assert spider.allowed_domains == {"blog.example.com", "contents.example.com"}


def test_init_path_focus():
spider = BaseSitemapSpider(
seed_urls=(
Expand Down
16 changes: 16 additions & 0 deletions data-connector-lib/test/dpk_connector/core/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
get_content_type,
get_etld1,
get_focus_path,
get_fqdn,
get_header_value,
get_mime_type,
is_allowed_path,
Expand Down Expand Up @@ -83,6 +84,21 @@ def test_get_etld1(url: str, expected: str):
assert get_etld1(url) == expected


@pytest.mark.parametrize(
"url,expected",
[
("http://www.example.com", "www.example.com"),
("https://www.example.co.uk", "www.example.co.uk"),
("http://www.example.com/path?query=string#fragment", "www.example.com"),
("http://localhost:8080/", ""),
("http://www.example.com:8080/", "www.example.com"),
("http://www.sub.example.com:8080/", "www.sub.example.com"),
],
)
def test_get_fqdn(url: str, expected: str):
assert get_fqdn(url) == expected


@pytest.mark.parametrize(
"url,expected",
[
Expand Down