Skip to content

fix(spark): drop the stray argument passed to _backend_config in the advanced-options example - #786

Merged
google-oss-prow[bot] merged 1 commit into
kubeflow:mainfrom
Anai-Guo:fix/spark-example-backend-config-arg
Oct 2, 2026
Merged

google-oss-prow[bot] merged 1 commit into
kubeflow:mainfrom
Anai-Guo:fix/spark-example-backend-config-arg

Conversation

@Anai-Guo

@Anai-Guo Anai-Guo commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Problem

examples/spark/spark_advanced_options.py defines the helper with no parameters:

def _backend_config():
    """Backend config; uses SPARK_TEST_NAMESPACE in CI."""
    return KubernetesBackendConfig(namespace=os.environ.get("SPARK_TEST_NAMESPACE", "default"))

Four of the five call sites invoke it correctly, but example 5 passes an argument:

line call
54 _backend_config()
101 _backend_config()
141 _backend_config()
185 _backend_config()
217 _backend_config("spark-production")

Line 217 raises:

TypeError: _backend_config() takes 0 positional arguments but 1 was given

main() wraps every example in a broad except Exception, so the user never
sees the TypeError itself — example 5 silently never runs and the script
prints the misleading advice instead:

Error: _backend_config() takes 0 positional arguments but 1 was given

Note: Make sure you have:
  1. Spark Operator installed with SparkConnect CRD
  2. Proper RBAC permissions configured
  ...

ALL EXAMPLES COMPLETE! is never printed. The in-cluster E2E run is unaffected
because SPARK_E2E_RUN_IN_CLUSTER=1 skips example 5, which is why this has gone
unnoticed in CI.

Fix

Drop the stray argument so line 217 matches the other four call sites.

I deliberately did not make _backend_config accept a namespace: the
namespace must stay environment-driven (SPARK_TEST_NAMESPACE) for the
in-cluster E2E run, and this example already names its session through the
options pattern it is demonstrating:

Name(_e2e_session_name("prod-etl-session")),

Verification

Signature replay against main (3febcd7):

signature: ()
  _backend_config()                    -> OK
  _backend_config("spark-production")  -> TypeError: too many positional arguments
call sites: line 54 (0 args), 101 (0), 141 (0), 185 (0), 217 (1 arg)

After the patch, no call site passes an argument. Diff is one line.

🤖 Generated with Claude Code

`_backend_config()` takes no parameters, but example 5
(`example_combined_options`) calls it as
`_backend_config("spark-production")`, which raises

    TypeError: _backend_config() takes 0 positional arguments but 1 was given

The other four call sites in this file all invoke it with no arguments.
Because `main()` wraps every example in a broad `except Exception`, the
failure surfaces as a misleading "make sure you have Spark Operator
installed / RBAC configured" message and example 5 never runs.

The namespace has to stay environment-driven so the in-cluster E2E run
keeps using `SPARK_TEST_NAMESPACE`, and this example already names its
session through the `Name(_e2e_session_name("prod-etl-session"))` option,
so the extra argument is simply dropped to match the sibling call sites.

Signed-off-by: Tai An <antai12232931@outlook.com>
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

🎉 Welcome to the Kubeflow SDK! 🎉

Thanks for opening your first PR! We're happy to have you as part of our community 🚀

Here's what happens next:

  • If you haven't already, please check out our Contributing Guide for repo-specific guidelines and the Kubeflow Contributor Guide for general community standards
  • Our team will review your PR soon! cc @kubeflow/kubeflow-sdk-team

Join the community:

Feel free to ask questions in the comments if you need any help or clarification!
Thanks again for contributing to Kubeflow! 🙏

@Anai-Guo Anai-Guo changed the title fix(examples/spark): drop the stray argument passed to _backend_config fix(spark): drop the stray argument passed to _backend_config in the advanced-options example Sep 7, 2026
@tariq-hasan

Copy link
Copy Markdown
Member

/ok-to-test
/retest

@tariq-hasan

Copy link
Copy Markdown
Member

Thanks!
/lgtm

@google-oss-prow google-oss-prow Bot added the lgtm label Sep 7, 2026
@RobuRishabh RobuRishabh moved this to Needs Triage in Kubeflow SDK Sep 21, 2026
@tariq-hasan tariq-hasan moved this from Needs Triage to Ready to Merge in Kubeflow SDK Sep 21, 2026

@andreyvelich andreyvelich left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm
/approve

@google-oss-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andreyvelich

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow
google-oss-prow Bot merged commit 3bf9a92 into kubeflow:main Oct 2, 2026
18 of 19 checks passed
@google-oss-prow google-oss-prow Bot added this to the v0.5 milestone Oct 2, 2026
@github-project-automation github-project-automation Bot moved this from Ready to Merge to Done in Kubeflow SDK Oct 2, 2026
digvijay-y pushed a commit to digvijay-y/sdk that referenced this pull request Oct 17, 2026
…advanced-options example (kubeflow#786)

`_backend_config()` takes no parameters, but example 5
(`example_combined_options`) calls it as
`_backend_config("spark-production")`, which raises

    TypeError: _backend_config() takes 0 positional arguments but 1 was given

The other four call sites in this file all invoke it with no arguments.
Because `main()` wraps every example in a broad `except Exception`, the
failure surfaces as a misleading "make sure you have Spark Operator
installed / RBAC configured" message and example 5 never runs.

The namespace has to stay environment-driven so the in-cluster E2E run
keeps using `SPARK_TEST_NAMESPACE`, and this example already names its
session through the `Name(_e2e_session_name("prod-etl-session"))` option,
so the extra argument is simply dropped to match the sibling call sites.

Signed-off-by: Tai An <antai12232931@outlook.com>
Signed-off-by: digvijay-y <144053736+digvijay-y@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants