Repository navigation
feat(spider-py): Add job submission support for MariaDB storage backend. #216
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
Changes from 235 commits
d2ac198
cc8a097
7f68bdc
2394649
6ff3e33
7ae8a78
56502f9
6f6becf
a74523d
2345d22
77ee494
38b86c8
3698339
369e9f1
90aa5a2
1769c95
edaa834
7faac8f
f494a90
eb01bb2
ff2fe1c
d476e42
850126d
65841a0
8564fc2
a6e7d29
573b448
6ad72c3
ba7c6e5
209acb1
406b514
66892f5
80f0a10
3186a42
0925938
6936a9b
7407090
6a95b24
f29dcba
453f209
d1011c7
a7dc642
a7d92c2
1c6213b
0f13d07
14d6326
55a727a
c98fea1
ef64454
09ba8ad
4e7237e
1f944d0
68992fc
1127b22
4c51c91
56878b2
bb428c9
944bd5e
d75bc15
4b7eca2
8ddbda7
0d998ec
6004d64
bcab5db
a50d25a
fbebb96
0322bd5
2a08155
dd29c23
570cf2f
9b2a175
32afe0a
0b62f73
7384158
70ef90a
1aae0cb
2f9b4a0
4e080ac
b09515c
bad9675
e009ff4
f11a3b6
a0407a7
aa9a8de
3c7e794
ca4ec29
b4d6576
23f31cd
0167ac0
5a9cb76
f447614
a84de83
f94460f
2d1b6fc
5a35122
061d101
451a80e
de149ee
d332dbe
80b6772
f2ebaa6
8c27b16
1b7ab77
b7f1884
a7aa6b0
95b98c8
e7b119c
8ffbd03
5765acf
427dd6b
a9c9e57
213d2fc
a1a86f3
97af71b
74f31e2
7d95706
96b5324
14ea6fb
854cd11
5565c9e
a9791e5
2504429
7ea4928
01dd0c6
c6ccce6
4cec09b
e022404
8894b90
01c9c64
7753153
dd59915
b86d47c
ac1d4ee
0d38c43
438f6e7
2caad5d
c6815ce
61e0651
8b13abd
50b2b25
519dcd8
8e2786b
a8c8641
9fabc44
e558f67
09d3fd0
ae2a935
008c6fb
cd60dc6
720265f
ebe64c6
404cf3b
3f9fe04
4d10f88
fe4121f
f3ac001
95b7773
a72e4a6
f289e5a
2e829bc
ec6802d
3685dd7
2862f01
03af08d
c3dda18
2a1f33e
0e4452c
3705ef3
fb8938a
9ed09be
ef827a3
220a2b0
56f7866
638d577
2bcc4a4
aa1723e
2469008
082476d
72239f7
e13acec
84a6256
c45eb0d
1a27b45
b505e06
44067ab
c81719b
899189e
ba1fd69
fe4b24b
8c0125a
beb4b5f
ff8bb01
43de342
6b06a30
8faa0c6
fe904b0
2f1c430
e1b1ac3
75793de
72a0920
2b5405a
fecdc68
3938934
eebcab7
5e2175a
bff3a95
e0fd981
08ce667
4dc9623
ff76756
b9607ae
75c8b76
b95af57
e81c509
d19fda2
afa7898
8e98c1e
fe306bc
d0fb7e2
6fe936d
82dab16
5ef3520
395d46c
b879134
1d6e28e
7d41718
67d3709
ef342cd
69adf5a
582abf6
ec2d1d4
413b3c8
5f8ae0e
9d19fff
6795c29
5b3dd17
d0c3049
383373f
86b318e
961018a
890b620
6384fbb
2e91ced
67a55ef
4ffec6d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
sitaowang1998 marked this conversation as resolved.
Resolved
|
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As discussed offline, let's add a sentence to explain how to set up the DB. We don't have the scheduler ready as it's not the integration test. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| """Spider core Driver module.""" | ||
|
|
||
| from uuid import UUID | ||
|
|
||
| DriverId = UUID |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| """Spider Storage package.""" | ||
|
|
||
| from .jdbc_url import JdbcParameters, parse_jdbc_url | ||
| from .mariadb_storage import MariaDBStorage | ||
| from .storage import Storage, StorageError | ||
|
|
||
| __all__ = [ | ||
| "JdbcParameters", | ||
| "MariaDBStorage", | ||
| "Storage", | ||
| "StorageError", | ||
| "parse_jdbc_url", | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| """JDBC URL module.""" | ||
|
|
||
| import re | ||
| from dataclasses import dataclass | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Use urllib.parse for robust query parsing and validate port range Manual splitting skips percent-decoding and multi-value handling. Also validate port bounds. Apply: -import re
+import re
+from urllib.parse import parse_qs
@@
- query = groups.get("query") or ""
- query_params = dict(param.split("=", 1) for param in query.split("&") if "=" in param)
+ query = groups.get("query") or ""
+ qmap = parse_qs(query, keep_blank_values=True, strict_parsing=False)
@@
- return JdbcParameters(
+ port = int(groups["port"]) if groups.get("port") else None
+ if port is not None and not (1 <= port <= 65535):
+ raise ValueError("Port out of range in JDBC URL")
+
+ return JdbcParameters(
protocol=protocol,
host=host,
- port=int(groups["port"]) if groups.get("port") else None,
+ port=port,
database=database,
- user=query_params.get("user"),
- password=query_params.get("password"),
+ user=(qmap.get("user", [None])[0]),
+ password=(qmap.get("password", [None])[0]),
)Also applies to: 41-43, 57-64 |
||
|
|
||
| @dataclass | ||
| class JdbcParameters: | ||
| """JDBC url parameters.""" | ||
|
|
||
| protocol: str | ||
| host: str | ||
| database: str | ||
| port: int | None = None | ||
| user: str | None = None | ||
| password: str | None = None | ||
|
|
||
|
|
||
| pattern = re.compile( | ||
| r"^(?P<protocol>[a-zA-Z][a-zA-Z0-9+.-]*(:[a-zA-Z0-9+.-]*)?)://" | ||
| r"(?P<host>([a-zA-Z0-9.-]+|\d{1,3}(?:\.\d{1,3}){3}))" | ||
| r"(?::(?P<port>\d+))?" | ||
| r"/(?P<database>[a-zA-Z0-9_\-]+)" | ||
| r"(?:\?(?P<query>[a-zA-Z0-9_\-=&]+))?" | ||
| ) | ||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
|
|
||
|
|
||
| def parse_jdbc_url(url: str) -> JdbcParameters: | ||
|
sitaowang1998 marked this conversation as resolved.
Resolved
|
||
| """ | ||
| Parses a JDBC URL. | ||
| :param url: | ||
| :return: | ||
|
sitaowang1998 marked this conversation as resolved.
Resolved
|
||
| :raises ValueError: If the JDBC URL is invalid. | ||
| """ | ||
| match = pattern.match(url) | ||
| if not match: | ||
| msg = f"Invalid JDBC URL: {url}" | ||
| raise ValueError(msg) | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not echo credentials in exceptions Error messages include the full URL, leaking passwords in logs. Redact sensitive query params before raising. Apply this diff to use a redaction helper in error paths: - match = pattern.match(url)
+ match = pattern.match(url)
if not match:
- msg = f"Invalid JDBC URL: {url}"
+ msg = f"Invalid JDBC URL: { _redact_url(url) }"
raise ValueError(msg)
@@
- if not protocol:
- msg = f"Protocol is required in JDBC URL: {url}"
+ if not protocol:
+ msg = f"Protocol is required in JDBC URL: { _redact_url(url) }"
raise ValueError(msg)
@@
- if not host:
- msg = f"Host is required in JDBC URL: {url}"
+ if not host:
+ msg = f"Host is required in JDBC URL: { _redact_url(url) }"
raise ValueError(msg)
@@
- if not database:
- msg = f"Database is required in JDBC URL: {url}"
+ if not database:
+ msg = f"Database is required in JDBC URL: { _redact_url(url) }"
raise ValueError(msg)Add this helper above def _redact_url(url: str) -> str:
base, _, query = url.partition("?")
if not query:
return url
redacted = []
for kv in query.split("&"):
k, eq, v = kv.partition("=")
if k.lower() in {"password", "pwd", "pass"} and eq:
redacted.append(f"{k}=***")
else:
redacted.append(kv)
return f"{base}?{'&'.join(redacted)}"🤖 Prompt for AI Agents |
||
| groups = match.groupdict() | ||
| query = groups.get("query") or "" | ||
| query_params = dict(param.split("=", 1) for param in query.split("&") if "=" in param) | ||
|
|
||
| protocol = groups.get("protocol") | ||
| if not protocol: | ||
| msg = f"Protocol is required in JDBC URL: {url}" | ||
| raise ValueError(msg) | ||
| host = groups.get("host") | ||
| if not host: | ||
| msg = f"Host is required in JDBC URL: {url}" | ||
| raise ValueError(msg) | ||
| database = groups.get("database") | ||
| if not database: | ||
| msg = f"Database is required in JDBC URL: {url}" | ||
| raise ValueError(msg) | ||
|
|
||
| return JdbcParameters( | ||
| protocol=protocol, | ||
| host=host, | ||
| port=int(groups["port"]) if groups.get("port") else None, | ||
| database=database, | ||
| user=query_params.get("user"), | ||
| password=query_params.get("password"), | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious: what's the plan for the storage tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean storage tests in GH workflows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Are we planning to add this to our workflows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There were talks about it, but no concrete plans. I don't think this is very hard to implement, but we do need to sync the test jdbc url across C++ and Python as they are different right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I think we should set it up ASAP. Let's prioritize this after ur're done with compression integration.