You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "sryza (via GitHub)" <gi...@apache.org> on 2025/06/04 15:00:10 UTC

[PR] Ensure structured_logging_style runs out of correct directory [spark]

sryza opened a new pull request, #51087:
URL: https://github.com/apache/spark/pull/51087

   ### What changes were proposed in this pull request?
   
   Currently, structured_logging_style.py assumes that it's being run from the `dev` directory. If it's run from the project root, it will scan all files in the parent directory, i.e. outside the project root.
   
   This change avoids that assumption, so `python dev/structured_logging_style.py` will still produce correct results.
   
   
   ### Why are the changes needed?
   
   Make it easier for developers to run the structured logging checks locally.
   
   ### Does this PR introduce _any_ user-facing change?
   <!--
   Note that it means *any* user-facing change including all aspects such as new features, bug fixes, or other behavior changes. Documentation-only updates are not considered user-facing changes.
   
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
   If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
   If no, write 'No'.
   -->
   
   No
   
   ### How was this patch tested?
   <!--
   If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
   If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
   If tests were not added, please describe why they were not added and/or why it was difficult to add.
   If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
   -->
   
   ```
   dev/structured_logging_style.py
   ```
   
   ### Was this patch authored or co-authored using generative AI tooling?
   <!--
   If generative AI tooling has been used in the process of authoring this patch, please include the
   phrase: 'Generated-by: ' followed by the name of the tool and its version.
   If no, write 'No'.
   Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
   -->
   
   No


-- 
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] Ensure structured_logging_style runs out of correct directory [spark]

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #51087: [MINOR] Ensure structured_logging_style runs out of correct directory
URL: https://github.com/apache/spark/pull/51087


-- 
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] Ensure structured_logging_style runs out of correct directory [spark]

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #51087:
URL: https://github.com/apache/spark/pull/51087#issuecomment-2948338607

   That's fine. no biggie. Let's just merge. I think it's because of the Python version (python 3.9) being used in the linter


-- 
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] Ensure structured_logging_style runs out of correct directory [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #51087:
URL: https://github.com/apache/spark/pull/51087#issuecomment-2945077071

   - https://github.com/apache/spark/actions/runs/15457270483/job/43511767165
   ![image](https://github.com/user-attachments/assets/cc10a5eb-90b1-466a-bc3e-f340c2ea1597)
   @sryza 


-- 
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] Ensure structured_logging_style runs out of correct directory [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #51087:
URL: https://github.com/apache/spark/pull/51087#discussion_r2126863874


##########
dev/structured_logging_style.py:
##########
@@ -41,7 +41,7 @@ def main():
 
     nonmigrated_files = {}
 
-    scala_files = glob.glob(os.path.join("../", "**", "*.scala"), recursive=True)
+    scala_files = glob.glob(os.path.join(__file__, "../", "**", "*.scala"), recursive=True)

Review Comment:
   Is this better?
   
   



-- 
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] Ensure structured_logging_style runs out of correct directory [spark]

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #51087:
URL: https://github.com/apache/spark/pull/51087#issuecomment-2942763830

   Reverted, and reopned.


-- 
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] Ensure structured_logging_style runs out of correct directory [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #51087:
URL: https://github.com/apache/spark/pull/51087#issuecomment-2945118236

   ```
   dev/lint-python 
   
   starting python compilation test...
   python compilation succeeded.
   
   starting black test...
   black checks failed:
   would reformat /Users/yangjie01/spark/dev/structured_logging_style.py
   
   Oh no! 💥 💔 💥
   1 file would be reformatted, 1155 files would be left unchanged.
   Please run 'dev/reformat-python' script.
   1
   ```
   
   ```
   dev/reformat-python
   
   reformatted /Users/yangjie01/spark/dev/structured_logging_style.py
   
   All done! ✨ 🍰 ✨
   1 file reformatted, 1155 files left unchanged.
   ```
   
   ```
   git diff
   
   diff --git a/dev/structured_logging_style.py b/dev/structured_logging_style.py
   index b7705ec9726..1196dbf4ac8 100755
   --- a/dev/structured_logging_style.py
   +++ b/dev/structured_logging_style.py
   @@ -41,7 +41,9 @@ def main():
    
        nonmigrated_files = {}
    
   -    scala_files = glob.glob(os.path.join(os.path.dirname(__file__), "../", "+", "*.scala"), recursive=True)
   +    scala_files = glob.glob(
   +        os.path.join(os.path.dirname(__file__), "../", "+", "*.scala"), recursive=True
   +    )
    
        for file in scala_files:
            skip_file = False
   ```
   
   @sryza  You need to refer to lines 79-101 in the file at https://github.com/apache/spark/blob/273a5e79ec73db2e1460bdf77ca77ebbb174e083/dev/spark-test-image/lint/Dockerfile to install the dependencies for lint.


-- 
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] Ensure structured_logging_style runs out of correct directory [spark]

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #51087: [MINOR] Ensure structured_logging_style runs out of correct directory
URL: https://github.com/apache/spark/pull/51087


-- 
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] Ensure structured_logging_style runs out of correct directory [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #51087:
URL: https://github.com/apache/spark/pull/51087#issuecomment-2942754924

   ![image](https://github.com/user-attachments/assets/48c6ac95-1787-4424-9ac5-bbc56544fcbd)
   


-- 
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] Ensure structured_logging_style runs out of correct directory [spark]

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #51087:
URL: https://github.com/apache/spark/pull/51087#issuecomment-2948339754

   Merged to master.


-- 
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] Ensure structured_logging_style runs out of correct directory [spark]

Posted by "sryza (via GitHub)" <gi...@apache.org>.
sryza commented on PR #51087:
URL: https://github.com/apache/spark/pull/51087#issuecomment-2944681996

   Hmm when I run `dev/reformat-python`, nothing changes, even after rebasing on master:
   
   <img width="551" alt="image" src="https://github.com/user-attachments/assets/beb6f665-e644-414f-ad97-686a6552a82b" />
   
   @LuciferYang what error black issue are you seeing?


-- 
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] Ensure structured_logging_style runs out of correct directory [spark]

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #51087:
URL: https://github.com/apache/spark/pull/51087#issuecomment-2942575416

   Merged to master.


-- 
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] Ensure structured_logging_style runs out of correct directory [spark]

Posted by "sryza (via GitHub)" <gi...@apache.org>.
sryza commented on PR #51087:
URL: https://github.com/apache/spark/pull/51087#issuecomment-2946126664

   Interesting – I'm also running with black 23.12.1. I just tried with python 3.11 and still couldn't reproduce the error.
   
   I wonder if somehow the line length setting in the the pyproject.toml isn't being picked up?
   ```
   [tool.black]
   # When changing the version, we have to update
   # GitHub workflow version and dev/reformat-python
   required-version = "23.12.1"
   line-length = 100
   ```
   
   That said, I just directly made the suggested change without running the script.
   
   Thanks for your help with investigating this @LuciferYang .


-- 
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] Ensure structured_logging_style runs out of correct directory [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #51087:
URL: https://github.com/apache/spark/pull/51087#discussion_r2126862514


##########
dev/structured_logging_style.py:
##########
@@ -41,7 +41,7 @@ def main():
 
     nonmigrated_files = {}
 
-    scala_files = glob.glob(os.path.join("../", "**", "*.scala"), recursive=True)
+    scala_files = glob.glob(os.path.join(__file__, "../", "**", "*.scala"), recursive=True)

Review Comment:
   ```suggestion
       scala_files = glob.glob(os.path.join(os.path.dirname(__file__), "..", "+", "*.scala"), recursive=True)
   ```



-- 
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] Ensure structured_logging_style runs out of correct directory [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #51087:
URL: https://github.com/apache/spark/pull/51087#discussion_r2126862514


##########
dev/structured_logging_style.py:
##########
@@ -41,7 +41,7 @@ def main():
 
     nonmigrated_files = {}
 
-    scala_files = glob.glob(os.path.join("../", "**", "*.scala"), recursive=True)
+    scala_files = glob.glob(os.path.join(__file__, "../", "**", "*.scala"), recursive=True)

Review Comment:
   ```suggestion
       scala_files = glob.glob(os.path.join(os.path.dirname(__file__), "../", "+", "*.scala"), recursive=True)
   ```



-- 
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] Ensure structured_logging_style runs out of correct directory [spark]

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #51087:
URL: https://github.com/apache/spark/pull/51087#issuecomment-2942762046

   oops
   


-- 
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] Ensure structured_logging_style runs out of correct directory [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #51087:
URL: https://github.com/apache/spark/pull/51087#issuecomment-2948306179

   > Interesting – I'm also running with black 23.12.1. I just tried with python 3.11 and still couldn't reproduce the error.
   > 
   > I wonder if somehow the line length setting in the the pyproject.toml isn't being picked up?
   > 
   > ```
   > [tool.black]
   > # When changing the version, we have to update
   > # GitHub workflow version and dev/reformat-python
   > required-version = "23.12.1"
   > line-length = 100
   > ```
   > 
   > That said, I just directly made the suggested change without running the script.
   
   Do you know the reason for the problem that @sryza  mentioned? @HyukjinKwon 
   
   
   
   


-- 
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