Skip to content
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

[SPARK-49097][INFRA] Add Python3 environment detection for the build_error_docs method in build_api_decs.rb #47590

Closed
wants to merge 3 commits into from

Conversation

wayneguow
Copy link
Contributor

@wayneguow wayneguow commented Aug 2, 2024

What changes were proposed in this pull request?

This PR aims to add Python3 environment detection for the build_orror_docs method in build_api_decs.rb.

Why are the changes needed?

Make the environment exception prompts more friendly for developers when generating documents.
Before:
image
After:
image

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass GA.

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the DOCS label Aug 2, 2024
@yaooqinn yaooqinn changed the title [SPARK-49097][INFRA] Add Python3 environment detection for the build_orror_docs method in build_api_decs.rb [SPARK-49097][INFRA] Add Python3 environment detection for the build_error_docs method in build_api_decs.rb Aug 2, 2024
raise("Missing python3 in your path, stopping error doc generation")
end

system("python3 '#{SPARK_PROJECT_ROOT}/docs/util/build-error-docs.py'") \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since users may not use python venv, I suggest changing to python3 here.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

It seems the failure is relevant to this change.
Could you make CI happy, @wayneguow ?

@wayneguow wayneguow changed the title [SPARK-49097][INFRA] Add Python3 environment detection for the build_error_docs method in build_api_decs.rb [WIP][SPARK-49097][INFRA] Add Python3 environment detection for the build_error_docs method in build_api_decs.rb Aug 3, 2024
@wayneguow wayneguow marked this pull request as draft August 3, 2024 07:29
@github-actions github-actions bot added the INFRA label Aug 3, 2024
@@ -872,6 +872,8 @@ jobs:
run: |
# We need this link because the jekyll build calls `python`.
ln -s "$(which python3.9)" "/usr/local/bin/python"
# We need this link to make sure `python3` points to `python3.9` which contains the prerequisite packages.
ln -s "$(which python3.9)" "/usr/local/bin/python3"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, there are multiple pythons in the ci env:

  • Python 3.9.19
  • Python 3.10.12
    And python3 points to Python 3.10.12.

@wayneguow wayneguow changed the title [WIP][SPARK-49097][INFRA] Add Python3 environment detection for the build_error_docs method in build_api_decs.rb [SPARK-49097][INFRA] Add Python3 environment detection for the build_error_docs method in build_api_decs.rb Aug 4, 2024
@wayneguow wayneguow marked this pull request as ready for review August 4, 2024 13:50
@wayneguow
Copy link
Contributor Author

It seems the failure is relevant to this change. Could you make CI happy, @wayneguow ?

Well, I found out the reason, because python3 currently does not point to the python3.9 with the installation packages we need.

@@ -872,6 +872,8 @@ jobs:
run: |
# We need this link because the jekyll build calls `python`.
ln -s "$(which python3.9)" "/usr/local/bin/python"
Copy link
Member

@dongjoon-hyun dongjoon-hyun Aug 4, 2024

Choose a reason for hiding this comment

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

Can we remove the above two lines from SPARK-46935, @wayneguow ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good advice, this link is indeed introduced in this PR #44971 and only used in this place.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you, @wayneguow and @HyukjinKwon .
Merged to master.

fusheng-rd pushed a commit to fusheng-rd/spark that referenced this pull request Aug 6, 2024
…_error_docs` method in `build_api_decs.rb`

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

This PR aims to add Python3 environment detection for the `build_orror_docs` method in `build_api_decs.rb`.

### Why are the changes needed?

Make the environment exception prompts more friendly for developers when generating documents.
Before:
<img width="1322" alt="image" src="https://github.com/user-attachments/assets/9f31c951-e63a-479a-9600-2b62e8ad9ddd">
After:
<img width="1379" alt="image" src="https://github.com/user-attachments/assets/b0841f42-237a-429e-8673-5254328c6dd2">

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

No.

### How was this patch tested?

Pass GA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#47590 from wayneguow/inf.

Authored-by: Wei Guo <guow93@gmail.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
attilapiros pushed a commit to attilapiros/spark that referenced this pull request Oct 4, 2024
…_error_docs` method in `build_api_decs.rb`

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

This PR aims to add Python3 environment detection for the `build_orror_docs` method in `build_api_decs.rb`.

### Why are the changes needed?

Make the environment exception prompts more friendly for developers when generating documents.
Before:
<img width="1322" alt="image" src="https://github.com/user-attachments/assets/9f31c951-e63a-479a-9600-2b62e8ad9ddd">
After:
<img width="1379" alt="image" src="https://github.com/user-attachments/assets/b0841f42-237a-429e-8673-5254328c6dd2">

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

No.

### How was this patch tested?

Pass GA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#47590 from wayneguow/inf.

Authored-by: Wei Guo <guow93@gmail.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
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.

3 participants