Skip to content

spark_submit_hook: resolve connection master URL construction#61528

Merged
eladkal merged 1 commit intoapache:mainfrom
blcksrx:blcksrx/spark_submit
Feb 25, 2026
Merged

spark_submit_hook: resolve connection master URL construction#61528
eladkal merged 1 commit intoapache:mainfrom
blcksrx:blcksrx/spark_submit

Conversation

@blcksrx
Copy link
Contributor

@blcksrx blcksrx commented Feb 6, 2026

spark_submit_hook: resolve connection master URL construction

Root Cause

When a Spark connection is created from a URI (e.g., spark://spark-master:7077), the _parse_from_uri method in connection.py extracts the scheme (spark://) and stores it as conn_type, while conn.host only contains the hostname (spark-master).

However, _resolve_connection in SparkSubmitHook assumed all connections were created via UI where conn_type is always spark and conn.host contains the full master URL with protocol (e.g., spark://host, k8s://https://host).

Impact

When using environment variables or URI format, the master URL was missing its protocol prefix:

Input Output (Before) Output (After)
spark://spark-master:7077 spark-master:7077 spark://spark-master:7077
k8s://https://k8s-host:443 https://k8s-host:443 k8s://https://k8s-host:443

Fix

Updated _resolve_connection to properly handle connections created from URI by using conn_type to reconstruct the protocol prefix when conn.host doesn't already contain ://.

closes: #56453

Copy link
Contributor

@Nataneljpwd Nataneljpwd left a comment

Choose a reason for hiding this comment

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

Looks good, a few comments need to be addressed first

@blcksrx blcksrx force-pushed the blcksrx/spark_submit branch from 1427565 to f2a6469 Compare February 6, 2026 16:03
Copy link
Contributor

@Nataneljpwd Nataneljpwd left a comment

Choose a reason for hiding this comment

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

Looks great!
1 comment left to resolve and I will approve

@blcksrx blcksrx force-pushed the blcksrx/spark_submit branch from f2a6469 to 79f86da Compare February 8, 2026 16:45
Copy link
Contributor

@Nataneljpwd Nataneljpwd left a comment

Choose a reason for hiding this comment

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

Looks great!
Great job!

@blcksrx blcksrx force-pushed the blcksrx/spark_submit branch 2 times, most recently from 816f979 to e31e33c Compare February 8, 2026 17:23
@blcksrx
Copy link
Contributor Author

blcksrx commented Feb 8, 2026

Looks great!
Great job!

Thanks, I just fixed it, could you please review it again?

Copy link
Contributor

@Nataneljpwd Nataneljpwd left a comment

Choose a reason for hiding this comment

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

1 small change is left

@blcksrx
Copy link
Contributor Author

blcksrx commented Feb 10, 2026

@potiuk Could you please review this PR?

Copy link
Contributor

@Nataneljpwd Nataneljpwd left a comment

Choose a reason for hiding this comment

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

Looks great!

@blcksrx blcksrx force-pushed the blcksrx/spark_submit branch from e31e33c to 266d24f Compare February 16, 2026 18:52
…ious protocols

Signed-off-by: Hossein Torabi <sayed.torabi@ing.com>
@blcksrx blcksrx force-pushed the blcksrx/spark_submit branch from 266d24f to 93e3484 Compare February 20, 2026 11:27
Copy link
Contributor

@nevcohen nevcohen left a comment

Choose a reason for hiding this comment

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

Just one thing I didn't fully understand, overall good job.

@eladkal eladkal merged commit 937be1d into apache:main Feb 25, 2026
86 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error when parsing spark master URL

5 participants