You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "ybapat (via GitHub)" <gi...@apache.org> on 2026/07/24 05:19:31 UTC
[PR] [MINOR][CONNECT] Remove redundant user_context.user_id assignment in execute_command methods [spark]
ybapat opened a new pull request, #57493:
URL: https://github.com/apache/spark/pull/57493
### What changes were proposed in this pull request?
Remove two redundant assignments of `req.user_context.user_id = self._user_id` from `SparkConnectClient.execute_command` and `SparkConnectClient.execute_command_as_iterator`.
Both methods call `_execute_plan_request_with_metadata()` to build the request, and that helper already sets `req.user_context.user_id` under the identical `if self._user_id:` guard. The subsequent reassignments in the two callers are dead code.
```python
# _execute_plan_request_with_metadata() already does this:
if self._user_id:
req.user_context.user_id = self._user_id
# These lines in execute_command / execute_command_as_iterator were redundant:
- if self._user_id:
- req.user_context.user_id = self._user_id
```
Relates to GitHub issue #56408.
### Why are the changes needed?
Dead code removal / readability improvement. Setting the same protobuf field to the same value twice in sequence is misleading — it implies the first assignment might not be sufficient when it is.
### Does this PR introduce _any_ user-facing change?
No. The resulting `req.user_context.user_id` value is identical before and after this change.
### How was this patch tested?
No behavior change; existing Spark Connect unit tests cover `execute_command` / `execute_command_as_iterator` request construction.
> This PR was created with the assistance of Claude (AI). Disclosed per Apache Spark contribution guidelines.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [MINOR][CONNECT] Remove redundant user_context.user_id assignment in execute_command methods [spark]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #57493:
URL: https://github.com/apache/spark/pull/57493#issuecomment-5085342318
Merged, thanks @ybapat and @uros-b
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [MINOR][CONNECT] Remove redundant user_context.user_id assignment in execute_command methods [spark]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang closed pull request #57493: [MINOR][CONNECT] Remove redundant user_context.user_id assignment in execute_command methods
URL: https://github.com/apache/spark/pull/57493
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [MINOR][CONNECT] Remove redundant user_context.user_id assignment in execute_command methods [spark]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #57493:
URL: https://github.com/apache/spark/pull/57493#issuecomment-5085340888
**Merge Summary:**
- merged into master https://github.com/apache/spark/commit/12f4f9bd8e38cc8048d7b747ffe38af9462ddc70
- merged into branch-4.x https://github.com/apache/spark/commit/52de4a925260ceaf9455a83d4ca50f2e021cddad
*Posted by `merge_spark_pr.py`*
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [MINOR][CONNECT] Remove redundant user_context.user_id assignment in execute_command methods [spark]
Posted by "ybapat (via GitHub)" <gi...@apache.org>.
ybapat commented on PR #57493:
URL: https://github.com/apache/spark/pull/57493#issuecomment-5081992699
Hi @uros-b, thank you for approving! CI is passing, do you need to manually merge it?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [MINOR][CONNECT] Remove redundant user_context.user_id assignment in execute_command methods [spark]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #57493:
URL: https://github.com/apache/spark/pull/57493#issuecomment-5085273173
LGTM. `_execute_plan_request_with_metadata()` already sets `req.user_context.user_id` under the identical guard, so the removed assignment writes the same value and is never used — pure dead code removal, no behavior change.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org