Skip to content

TEZ-4666: Migrate tez-tools python scripts to python3 - #444

Merged
abstractdog merged 2 commits into
apache:masterfrom
Aggarwal-Raghav:TEZ-4666
Jan 16, 2026
Merged

TEZ-4666: Migrate tez-tools python scripts to python3#444
abstractdog merged 2 commits into
apache:masterfrom
Aggarwal-Raghav:TEZ-4666

Conversation

@Aggarwal-Raghav

@Aggarwal-RaghavAggarwal-Raghav commented Nov 21, 2025

Copy link
Copy Markdown
Contributor

BEFORE FIX:
swimlane_amparser_error
coutner_diff_error

AFTER FIX:
swimlane-working
Counter_diff_fix

with open(dag_json_file1) as data_file:
file1_dag_json = json.load(data_file)["dag"] if file1_using_dag_json else json.load(data_file)
counters = file1_dag_json['otherinfo']['counters']
for group in counters['counterGroups']:

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Added safe check because of such errors
Image

@abstractdog
abstractdog self-requested a review November 21, 2025 08:54
@abstractdog

Copy link
Copy Markdown
Contributor

this is awesome @Aggarwal-Raghav , thanks! +1

@Aggarwal-Raghav

Copy link
Copy Markdown
ContributorAuthor

The goal was to keep the changes at minimum and convert to python3. Otherwise there is scope of improvement in scripts by using argparser instead of sys.argv, pathlib instead of os.path.

@abstractdog

Copy link
Copy Markdown
Contributor

The goal was to keep the changes at minimum and convert to python3. Otherwise there is scope of improvement in scripts by using argparser instead of sys.argv, pathlib instead of os.path.

yeah, small refactors are always welcome, absolutely fit this PR

@tez-yetus

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec31m 27sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+0 🆗detsecrets0m 0sdetect-secrets was not available.
+0 🆗shelldocs0m 0sShelldocs was not available.
+1 💚@author0m 0sThe patch does not contain any @author tags.
_ master Compile Tests _
-1 ❌pylint0m 1s/branch-pylint-stderr.txtError running pylint. Please check pylint stderr files.
_ Patch Compile Tests _
+1 💚codespell0m 4sNo new issues.
-1 ❌blanks0m 0s/blanks-eol.txtThe patch has 7 line(s) that end in blanks. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
-1 ❌blanks0m 0s/blanks-tabs.txtThe patch 84 line(s) with tabs.
+1 💚markdownlint0m 2sThe patch generated 0 new + 45 unchanged - 1 fixed = 45 total (was 46)
-1 ❌pylint0m 2s/patch-pylint-stderr.txtError running pylint. Please check pylint stderr files.
+1 💚pylint0m 2sNo new issues.
+1 💚shellcheck0m 0sNo new issues.
_ Other Tests _
-1 ❌asflicense0m 51s/results-asflicense.txtThe patch generated 1 ASF License warnings.
33m 24s
SubsystemReport/Notes
DockerClientAPI=1.52 ServerAPI=1.52 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-444/1/artifact/out/Dockerfile
GITHUB PR#444
Optional Testsdupname asflicense codespell detsecrets pylint markdownlint shellcheck shelldocs
unameLinux d201e749b075 5.15.0-153-generic #163-Ubuntu SMP Thu Aug 7 16:37:18 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personality/home/jenkins/jenkins-home/workspace/tez-multibranch_PR-444/src/.yetus/personality.sh
git revisionmaster / 92119cb
Max. process+thread count52 (vs. ulimit of 5500)
modulesC: tez-tools U: tez-tools
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-444/1/console
versionsgit=2.43.0 maven=3.8.7 codespell=2.0.0 markdownlint=0.23.2 shellcheck=0.7.1
Powered byApache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

@maheshrajus

Copy link
Copy Markdown
Contributor

LGTM
@Aggarwal-Raghav can you please check the above failures in yetus report. thanks !

@Aggarwal-Raghav

Copy link
Copy Markdown
ContributorAuthor

LGTM @Aggarwal-Raghav can you please check the above failures in yetus report. thanks !

thanks for the review @maheshrajus , sure, I'll fix the failures.

@Aggarwal-Raghav

Copy link
Copy Markdown
ContributorAuthor

@maheshrajus@abstractdog , it seems that the pylint is not working in yetus based on /branch-pylint-stderr.txt ,
As i ran the pylint in local, there are lot of issues unrelated to the change. I can surely check/fix them along with shellcheck, blanks, asfheader etc.

Also, 1 major flaw in python scripts are, tabs are not converted to spaces and has un-even indentation.

Python's official style guide, PEP 8, strongly recommends using spaces for indentation, specifically four consecutive spaces per indentation level. While Python technically allows both tabs and spaces for indentation, and even disallows mixing them within the same file, spaces are the widely preferred and recommended convention

Scripts are old and requires some decent changes. Personally, I'm ready and committed to fix them. Will also accomodate that pathlib and argparser changes as well. I'll use both https://github.com/astral-sh/ruff and pylint to ensure scripts are of good quality.

Screenshot 2025-11-21 at 3 28 10 PM

@abstractdog

Copy link
Copy Markdown
Contributor

@maheshrajus@abstractdog , it seems that the pylint is not working in yetus based on /branch-pylint-stderr.txt , As i ran the pylint in local, there are lot of issues unrelated to the change. I can surely check/fix them along with shellcheck, blanks, asfheader etc.

Also, 1 major flaw in python scripts are, tabs are not converted to spaces and has un-even indentation.

Python's official style guide, PEP 8, strongly recommends using spaces for indentation, specifically four consecutive spaces per indentation level. While Python technically allows both tabs and spaces for indentation, and even disallows mixing them within the same file, spaces are the widely preferred and recommended convention

Scripts are old and requires some decent changes. Personally, I'm ready and committed to fix them. Will also accomodate that pathlib and argparser changes as well. I'll use both https://github.com/astral-sh/ruff and pylint to ensure scripts are of good quality.

Screenshot 2025-11-21 at 3 28 10 PM

thanks a lot for taking a look at this!
it's fine to fix all these things in follow-up tickets, mainly because they tend to fall into different categories:

  1. python problems with our code (a lot)
  2. pylint problems that needs yetus configuration maybe

@Aggarwal-Raghav

Aggarwal-Raghav commented Nov 21, 2025

Copy link
Copy Markdown
ContributorAuthor

i have removed the tabs with space (just indentation change). that should fix the /blanks-eol.txt/blanks-tabs.txt and requirements.txt for now

@tez-yetus

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 27sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+0 🆗detsecrets0m 0sdetect-secrets was not available.
+0 🆗shelldocs0m 0sShelldocs was not available.
+1 💚@author0m 1sThe patch does not contain any @author tags.
_ master Compile Tests _
-1 ❌pylint0m 1s/branch-pylint-stderr.txtError running pylint. Please check pylint stderr files.
_ Patch Compile Tests _
+1 💚codespell0m 4sNo new issues.
+1 💚blanks0m 0sThe patch has no blanks issues.
+1 💚markdownlint0m 3sThe patch generated 0 new + 45 unchanged - 1 fixed = 45 total (was 46)
-1 ❌pylint0m 2s/patch-pylint-stderr.txtError running pylint. Please check pylint stderr files.
+1 💚pylint0m 3sNo new issues.
+1 💚shellcheck0m 0sThe patch generated 0 new + 0 unchanged - 1 fixed = 0 total (was 1)
_ Other Tests _
-1 ❌asflicense0m 49s/results-asflicense.txtThe patch generated 1 ASF License warnings.
2m 21s
SubsystemReport/Notes
DockerClientAPI=1.52 ServerAPI=1.52 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-444/2/artifact/out/Dockerfile
GITHUB PR#444
Optional Testsdupname asflicense codespell detsecrets pylint markdownlint shellcheck shelldocs
unameLinux 875f66e6fb1c 5.15.0-161-generic #171-Ubuntu SMP Sat Oct 11 08:17:01 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personality/home/jenkins/jenkins-home/workspace/tez-multibranch_PR-444/src/.yetus/personality.sh
git revisionmaster / d317d55
Max. process+thread count51 (vs. ulimit of 5500)
modulesC: tez-tools U: tez-tools
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-444/2/console
versionsgit=2.43.0 maven=3.8.7 codespell=2.0.0 markdownlint=0.23.2 shellcheck=0.7.1
Powered byApache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

@Aggarwal-Raghav

Copy link
Copy Markdown
ContributorAuthor

Earlier pylint rating:

logsplit.py: Your code has been rated at 6.85/10
swimlane.py: Your code has been rated at 0.00/10
counter-diff.py: Your code has been rated at 0.00/10
amlogparser.py: Your code has been rated at 0.00/10

pylint rating after reformatting:

logsplit.py: Your code has been rated at 6.99/10
swimlane.py: Your code has been rated at 6.17/10
counter-diff.py: Your code has been rated at 8.35/10
amlogparser.py: Your code has been rated at 6.71/10

@abstractdog

abstractdog commented Jan 16, 2026

Copy link
Copy Markdown
Contributor

patch looks good to me now, thanks @Aggarwal-Raghav, I'm merging this soon
can you please create a follow-up for the pylint issue?

@abstractdog
abstractdog merged commit 6dc889e into apache:masterJan 16, 2026
@Aggarwal-Raghav

Aggarwal-Raghav commented Jan 16, 2026

Copy link
Copy Markdown
ContributorAuthor

patch looks good to me now, thanks @Aggarwal-Raghav, I'm merging this soon can you please create a follow-up for the pylint issue?

Thanks for merging this @abstractdog . I'll create the follow-up JIRA to fix pylint errors and maybe further improving the scripts.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Aggarwal-Raghav@abstractdog@tez-yetus@maheshrajus