Skip to content

[SPARK-41705][CONNECT] Move generate_protos.sh to dev/#39211

Closed
tedyu wants to merge 1 commit intoapache:masterfrom
tedyu:gen-proto
Closed

[SPARK-41705][CONNECT] Move generate_protos.sh to dev/#39211
tedyu wants to merge 1 commit intoapache:masterfrom
tedyu:gen-proto

Conversation

@tedyu
Copy link
Contributor

@tedyu tedyu commented Dec 25, 2022

What changes were proposed in this pull request?

This PR moves generate_protos.sh from connector/connect/dev to dev directory.

Why are the changes needed?

connector/connect/dev only contains one script. Moving generate_protos.sh to dev follows practice for other scripts.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing test suite.

@tedyu
Copy link
Contributor Author

tedyu commented Dec 25, 2022

cc @HyukjinKwon

@tedyu tedyu force-pushed the gen-proto branch 2 times, most recently from cec9cd6 to 5d17794 Compare December 25, 2022 15:48
@tedyu
Copy link
Contributor Author

tedyu commented Dec 25, 2022

KubernetesSuite.scala failure may not be related to the PR.

pushd /__w/connector/connect/common/src/main
/__w/spark/spark/dev/generate_protos.sh: line 39: pushd: /__w/connector/connect/common/src/main: No such file or directory

I tried to account for SPARK_HOME:

pushd ${SPARK_HOME}/connector/connect/common/src/main

Though the above doesn't seem to work.

@grundprinzip
Copy link
Contributor

I don't think it's worth moving it atm. It has a clear relationship to the contained code in the connect package.

@HyukjinKwon
Copy link
Member

Actually maybe now it's time to consider moving these (including README.md) together since we're going to package them together (#38477). The reason is that all dev related scripts are located there e.g., dev/reformat-python, etc.

@HyukjinKwon
Copy link
Member

Let's also fix dev/check-codegen-python.py script together to fix up the linter.

@HyukjinKwon
Copy link
Member

Should also better file a JIRA.

@tedyu tedyu changed the title [CONNECT][MINOR] Move generate_protos.sh to dev/ [SPARK-41705][CONNECT][MINOR] Move generate_protos.sh to dev/ Dec 26, 2022
Copy link
Member

Choose a reason for hiding this comment

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

Since we already did cd "$SPARK_HOME" above, I guess we won't need this change anyway (?)

@HyukjinKwon HyukjinKwon changed the title [SPARK-41705][CONNECT][MINOR] Move generate_protos.sh to dev/ [SPARK-41705][CONNECT] Move generate_protos.sh to dev/ Dec 26, 2022
@zhengruifeng
Copy link
Contributor

the code gen checker failed:

Traceback (most recent call last):
  File "./dev/check-codegen-python.py", line 84, in <module>
    check_connect_protos()
  File "./dev/check-codegen-python.py", line 49, in check_connect_protos
    run_cmd(f"{SPARK_HOME}/dev/generate_protos.sh {tmp}")
  File "./dev/check-codegen-python.py", line 43, in run_cmd
    return subprocess.check_output(cmd.split(" ")).decode("utf-8")
  File "/usr/lib/python3.8/subprocess.py", line 415, in check_output
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
  File "/usr/lib/python3.8/subprocess.py", line 516, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['/__w/spark/spark/dev/generate_protos.sh', '/tmp/tmpdk4qg3nj']' returned non-zero exit status 1.

@HyukjinKwon
Copy link
Member

will take this over with moving related README.md together.

@HyukjinKwon
Copy link
Member

#39338. I will add you as a co-author there.

@HyukjinKwon HyukjinKwon closed this Jan 2, 2023
HyukjinKwon added a commit that referenced this pull request Jan 2, 2023
…and script to dev/ and Python documentation

### What changes were proposed in this pull request?

This PR takes over #39211 and #38477 that proposes:

- Move `connector/connect/dev/generate_protos.sh` → `dev/generate_protos.sh` to be consistent with other places
- Move Python-specific development guides into `python/docs/source/development/testing.rst`

### Why are the changes needed?

To keep the project structure and documentation consistent.

### Does this PR introduce _any_ user-facing change?

Python-specific development guides for Spark Connect will be added in https://spark.apache.org/docs/latest/api/python/development/testing.html.

### How was this patch tested?

I manually tested:

```
./dev/generate_protos.sh
./dev/check-codegen-python.py
```

I also manually verified the Python documentation.

Closes #39338 from HyukjinKwon/SPARK-41705.

Lead-authored-by: Hyukjin Kwon <gurwls223@apache.org>
Co-authored-by: Ted Yu <yuzhihong@gmail.com>
Co-authored-by: Rui Wang <rui.wang@databricks.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
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.

4 participants