TEZ-4743: Refactor ProfileServlet and ProfileOutputServlet to use standard response.sendError API by Aggarwal-Raghav · Pull Request #527 · apache/tez

Aggarwal-Raghav · 2026-07-19T11:27:52Z

Replaced manual error writing with the standard, built-in response.sendError(...) API in both servlets
Removed the obsolete writeMessage() and PrintStream logic
If the ?file= parameter is missing, the servlet now correctly returns a 400 Bad Request instead of incorrectly returning a 200 OK success code

tez-yetus · 2026-07-19T11:51:28Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	4m 15s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
-1 ❌	test4tests	0m 0s		The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
			_ master Compile Tests _
+1 💚	mvninstall	6m 16s		master passed
-1 ❌	compile	2m 49s	/branch-compile-root.txt	root in master failed.
-0 ⚠️	checkstyle	0m 27s	/buildtool-branch-checkstyle-tez-common.txt	The patch fails to run checkstyle in tez-common
+1 💚	javadoc	0m 31s		master passed
+0 🆗	spotbugs	1m 4s		tez-common in master has 13 extant spotbugs warnings.
			_ Patch Compile Tests _
-1 ❌	mvninstall	1m 17s	/patch-mvninstall-root.txt	root in the patch failed.
+1 💚	codespell	1m 59s		No new issues.
-1 ❌	compile	0m 22s	/patch-compile-root.txt	root in the patch failed.
-1 ❌	javac	0m 22s	/patch-compile-root.txt	root in the patch failed.
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 35s	/buildtool-patch-checkstyle-tez-common.txt	The patch fails to run checkstyle in tez-common
-1 ❌	javadoc	0m 31s	/patch-javadoc-tez-common.txt	tez-common in the patch failed.
-1 ❌	spotbugs	0m 44s	/patch-spotbugs-tez-common.txt	tez-common in the patch failed.
			_ Other Tests _
-1 ❌	unit	0m 14s	/patch-unit-root.txt	root in the patch failed.
+0 🆗	asflicense	0m 16s		ASF License check generated no output?
		23m 8s

Subsystem	Report/Notes
Docker	ClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-527/1/artifact/out/Dockerfile
Optional Tests	dupname compile unit asflicense javac javadoc spotbugs checkstyle codespell detsecrets
uname	Linux c478e300147c 5.15.0-181-generic #191-Ubuntu SMP Fri May 22 19:09:02 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	tez-personality.sh
git revision	master / `b9d8c0f`
Default Java	Eclipse Adoptium-21.0.11+10-LTS
Test Results	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-527/1/testReport/
Max. process+thread count	116 (vs. ulimit of 5500)
modules	C: tez-common U: tez-common
Console output	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-527/1/console
versions	git=2.43.0 maven=3.9.15 spotbugs=4.9.3 codespell=2.4.1
Powered by	Apache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

abstractdog · 2026-07-22T13:59:49Z

this looks good so far 👍 assuming a rebase

Aggarwal-Raghav · 2026-07-22T14:01:00Z

Will do in an hour

tez-yetus · 2026-07-23T07:22:39Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 11s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s		The patch appears to include 1 new or modified test files.
			_ master Compile Tests _
+0 🆗	mvndep	0m 36s		Maven dependency ordering for branch
+1 💚	mvninstall	4m 41s		master passed
+1 💚	compile	4m 2s		master passed
+1 💚	checkstyle	1m 0s		master passed
+1 💚	javadoc	0m 55s		master passed
+0 🆗	spotbugs	0m 53s		tez-common in master has 13 extant spotbugs warnings.
+0 🆗	spotbugs	0m 51s		tez-tests in master has 6 extant spotbugs warnings.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 10s		Maven dependency ordering for patch
+1 💚	mvninstall	3m 47s		the patch passed
+1 💚	codespell	1m 37s		No new issues.
+1 💚	compile	4m 3s		the patch passed
+1 💚	javac	4m 3s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 53s		the patch passed
+1 💚	javadoc	0m 53s		the patch passed
-1 ❌	spotbugs	1m 5s	/new-spotbugs-tez-common.html	tez-common generated 1 new + 13 unchanged - 0 fixed = 14 total (was 13)
			_ Other Tests _
+1 💚	unit	78m 19s		root in the patch passed.
+1 💚	asflicense	0m 58s		The patch does not generate ASF License warnings.
		107m 51s

Reason	Tests
SpotBugs	module:tez-common
	HTTP parameter written to Servlet error page in org.apache.tez.common.web.ProfileOutputServlet.doGet(HttpServletRequest, HttpServletResponse) At ProfileOutputServlet.java:error page in org.apache.tez.common.web.ProfileOutputServlet.doGet(HttpServletRequest, HttpServletResponse) At ProfileOutputServlet.java:[line 61]

Subsystem	Report/Notes
Docker	ClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-527/3/artifact/out/Dockerfile
Optional Tests	dupname compile unit asflicense javac javadoc spotbugs checkstyle codespell detsecrets
uname	Linux 4446ddf0551d 5.15.0-181-generic #191-Ubuntu SMP Fri May 22 19:09:02 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	tez-personality.sh
git revision	master / `dd8137f`
Default Java	Eclipse Adoptium-21.0.11+10-LTS
Test Results	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-527/3/testReport/
Max. process+thread count	1498 (vs. ulimit of 5500)
modules	C: tez-common tez-tests U: .
Console output	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-527/3/console
versions	git=2.43.0 maven=3.9.15 spotbugs=4.9.3 codespell=2.4.1
Powered by	Apache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

Aggarwal-Raghav · 2026-07-23T07:39:34Z

There is a delay in github webhooks. I forced pushed to fix the spotbugs vulnerability but its not getting reflected in PR.

tez-yetus · 2026-07-23T09:28:23Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 11s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s		The patch appears to include 1 new or modified test files.
			_ master Compile Tests _
+0 🆗	mvndep	0m 33s		Maven dependency ordering for branch
+1 💚	mvninstall	4m 38s		master passed
+1 💚	compile	4m 1s		master passed
+1 💚	checkstyle	0m 59s		master passed
+1 💚	javadoc	0m 55s		master passed
+0 🆗	spotbugs	0m 53s		tez-common in master has 13 extant spotbugs warnings.
+0 🆗	spotbugs	0m 50s		tez-tests in master has 6 extant spotbugs warnings.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 10s		Maven dependency ordering for patch
+1 💚	mvninstall	3m 42s		the patch passed
+1 💚	codespell	1m 37s		No new issues.
+1 💚	compile	4m 3s		the patch passed
+1 💚	javac	4m 3s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 53s		the patch passed
+1 💚	javadoc	0m 54s		the patch passed
+1 💚	spotbugs	2m 5s		the patch passed
			_ Other Tests _
-1 ❌	unit	75m 10s	/patch-unit-root.txt	root in the patch passed.
+1 💚	asflicense	1m 0s		The patch does not generate ASF License warnings.
		104m 30s

Reason	Tests
Failed junit tests	tez.dag.app.TestSpeculation

Subsystem	Report/Notes
Docker	ClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-527/4/artifact/out/Dockerfile
Optional Tests	dupname compile unit asflicense javac javadoc spotbugs checkstyle codespell detsecrets
uname	Linux fe54da384fba 5.15.0-181-generic #191-Ubuntu SMP Fri May 22 19:09:02 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	tez-personality.sh
git revision	master / `dd8137f`
Default Java	Eclipse Adoptium-21.0.11+10-LTS
Test Results	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-527/4/testReport/
Max. process+thread count	2128 (vs. ulimit of 5500)
modules	C: tez-common tez-tests U: .
Console output	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-527/4/console
versions	git=2.43.0 maven=3.9.15 spotbugs=4.9.3 codespell=2.4.1
Powered by	Apache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

…ndard response.sendError API - Replaced manual error writing with the standard, built-in response.sendError(...) API in both servlets - Removed the obsolete writeMessage() and PrintStream logic - If the `?file=` parameter is missing, the servlet now correctly returns a 400 Bad Request instead of incorrectly returning a 200 OK success code

tez-yetus · 2026-07-25T16:59:23Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 12s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s		The patch appears to include 1 new or modified test files.
			_ master Compile Tests _
+0 🆗	mvndep	0m 36s		Maven dependency ordering for branch
+1 💚	mvninstall	4m 37s		master passed
+1 💚	compile	4m 6s		master passed
+1 💚	checkstyle	0m 58s		master passed
+1 💚	javadoc	0m 57s		master passed
+0 🆗	spotbugs	0m 53s		tez-common in master has 13 extant spotbugs warnings.
+0 🆗	spotbugs	0m 51s		tez-tests in master has 6 extant spotbugs warnings.
			_ Patch Compile Tests _
+0 🆗	mvndep	0m 9s		Maven dependency ordering for patch
+1 💚	mvninstall	3m 46s		the patch passed
+1 💚	codespell	1m 36s		No new issues.
+1 💚	compile	4m 4s		the patch passed
+1 💚	javac	4m 4s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 54s		the patch passed
+1 💚	javadoc	0m 54s		the patch passed
+1 💚	spotbugs	2m 4s		the patch passed
			_ Other Tests _
+1 💚	unit	73m 42s		root in the patch passed.
+1 💚	asflicense	1m 0s		The patch does not generate ASF License warnings.
		103m 16s

Subsystem	Report/Notes
Docker	ClientAPI=1.55 ServerAPI=1.55 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-527/6/artifact/out/Dockerfile
Optional Tests	dupname compile unit asflicense javac javadoc spotbugs checkstyle codespell detsecrets
uname	Linux 8f8181295d72 5.15.0-181-generic #191-Ubuntu SMP Fri May 22 19:09:02 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	tez-personality.sh
git revision	master / `dd8137f`
Default Java	Eclipse Adoptium-21.0.11+10-LTS
Test Results	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-527/6/testReport/
Max. process+thread count	2127 (vs. ulimit of 5500)
modules	C: tez-common tez-tests U: .
Console output	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-527/6/console
versions	git=2.43.0 maven=3.9.15 spotbugs=4.9.3 codespell=2.4.1
Powered by	Apache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

Aggarwal-Raghav · 2026-07-25T17:00:19Z

Addressed review comment @abstractdog

abstractdog

LGTM

Aggarwal-Raghav mentioned this pull request Jul 21, 2026
TEZ-4741: ProfileOutputServlet: require instrumentation ACL to read profiler output #526
Merged

Aggarwal-Raghav force-pushed the TEZ-4743 branch 2 times, most recently from fe925d5 to 811deb4Compare July 23, 2026 05:34

Aggarwal-Raghav force-pushed the TEZ-4743 branch 2 times, most recently from 811deb4 to 582e485Compare July 23, 2026 07:49

Aggarwal-Raghav force-pushed the TEZ-4743 branch from 3e1404b to d80c8d0Compare July 23, 2026 13:56

This comment was marked as outdated.
Sign in to view

abstractdog reviewed Jul 25, 2026
View reviewed changes

Comment threadtez-common/src/main/java/org/apache/tez/common/web/ProfileServlet.java Outdated

Aggarwal-Raghav force-pushed the TEZ-4743 branch from d80c8d0 to f198215Compare July 25, 2026 15:15

Aggarwal-Raghav requested a review from abstractdog July 27, 2026 15:17

abstractdog approved these changes Jul 27, 2026
View reviewed changes

abstractdog merged commit efab6a1 into apache:masterJul 27, 2026
6 checks passed

Navigation Menu

Uh oh!

TEZ-4743: Refactor ProfileServlet and ProfileOutputServlet to use standard response.sendError API - #527

TEZ-4743: Refactor ProfileServlet and ProfileOutputServlet to use standard response.sendError API#527
abstractdog merged 1 commit into
apache:masterfrom
Aggarwal-Raghav:TEZ-4743

Aggarwal-Raghav commented Jul 19, 2026

Uh oh!

tez-yetus commented Jul 19, 2026

Uh oh!

abstractdog commented Jul 22, 2026

Uh oh!

Aggarwal-Raghav commented Jul 22, 2026

Uh oh!

tez-yetus commented Jul 23, 2026

Uh oh!

Aggarwal-Raghav commented Jul 23, 2026

Uh oh!

tez-yetus commented Jul 23, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

tez-yetus commented Jul 25, 2026

Uh oh!

Aggarwal-Raghav commented Jul 25, 2026

Uh oh!

abstractdog left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Aggarwal-Raghav commented Jul 19, 2026

Uh oh!

tez-yetus commented Jul 19, 2026

Uh oh!

abstractdog commented Jul 22, 2026

Uh oh!

Aggarwal-Raghav commented Jul 22, 2026

Uh oh!

tez-yetus commented Jul 23, 2026

Uh oh!

Aggarwal-Raghav commented Jul 23, 2026

Uh oh!

tez-yetus commented Jul 23, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

tez-yetus commented Jul 25, 2026

Uh oh!

Aggarwal-Raghav commented Jul 25, 2026

Uh oh!

abstractdog left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants