Skip to content

Conversation

@HuwCampbell
Copy link
Contributor

@HuwCampbell HuwCampbell commented Mar 15, 2024

What changes were proposed in this pull request?

Like SPARK-24553, this PR aims to fix redirect issues (incorrect 302) when one is using proxy settings. Change the generated link to be consistent with other links and include a trailing slash

Why are the changes needed?

When using a proxy, an invalid redirect is issued if this is not included

Does this PR introduce any user-facing change?

Only that people will be able to use these links if they are using a proxy

How was this patch tested?

With a proxy installed I went to the location this link would generate and could go to the page, when it redirects with the link as it exists.

Edit: Further tested by building a version of our application with this patch applied, the links work now.

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

No.

Page with working link
Screenshot 2024-03-18 at 4 45 27 PM

Goes correctly to
Screenshot 2024-03-18 at 4 45 36 PM

Before it would redirect and we'd get a 404.

image

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.

Thank you for making a PR, @HuwCampbell . Please create a new JIRA issue because you cannot make a follow-up PR for the released JIRA issue. Your new JIRA needs to have different Fix Versions.

@HuwCampbell
Copy link
Contributor Author

I've opened https://issues.apache.org/jira/browse/SPARK-47434

@HuwCampbell HuwCampbell changed the title [Spark-24553][WEB-UI][FOLLOWUP] Fix link for StreamingQueryPage statistics [Spark-47434][WEB-UI][FOLLOWUP] Fix link for StreamingQueryPage statistics Mar 17, 2024
@dongjoon-hyun
Copy link
Member

Thank you, @HuwCampbell .

@dongjoon-hyun
Copy link
Member

Could you enable GitHub Action on your repository? It seems that CI is not running.

@dongjoon-hyun
Copy link
Member

Also, cc @yaooqinn too.

Related to [Spark-24553]

This causes redirect issues when one is using proxy settings.
@dongjoon-hyun dongjoon-hyun changed the title [Spark-47434][WEB-UI][FOLLOWUP] Fix link for StreamingQueryPage statistics [SPARK-47434][WEBUI] Fix link for StreamingQueryPage statistics Mar 17, 2024
@dongjoon-hyun dongjoon-hyun changed the title [SPARK-47434][WEBUI] Fix link for StreamingQueryPage statistics [SPARK-47434][WEBUI] Fix statistics link in StreamingQueryPage Mar 17, 2024
@yaooqinn
Copy link
Member

With a proxy installed I went to the location this link would generate and could go to the page, when it redirects with the link as it exists.

Would you mind providing before-and-after screenshots in the PR desc?

@HuwCampbell
Copy link
Contributor Author

There's not a lot to show.

Before the link didn't work (we have a proxy setup on our application and it went to https://our-domain/StreamingQuery/statistics).

Now the link works.
Screenshot 2024-03-18 at 4 45 27 PM

Goes correctly to
Screenshot 2024-03-18 at 4 45 36 PM

@dongjoon-hyun
Copy link
Member

Ya, we understand. That's enough. Please attach them to the PR description, @HuwCampbell .

@dongjoon-hyun
Copy link
Member

In addition, you can add another screenshot which shows that the previous link doesn't work.

@HuwCampbell
Copy link
Contributor Author

Ok

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, @HuwCampbell and @yaooqinn .
Merged to master/3.5/3.4.

dongjoon-hyun pushed a commit that referenced this pull request Mar 18, 2024
### What changes were proposed in this pull request?

Like SPARK-24553, this PR aims to fix redirect issues (incorrect 302) when one is using proxy settings. Change the generated link to be consistent with other links and include a trailing slash

### Why are the changes needed?

When using a proxy, an invalid redirect is issued if this is not included

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

Only that people will be able to use these links if they are using a proxy

### How was this patch tested?

With a proxy installed I went to the location this link would generate and could go to the page, when it redirects with the link as it exists.

Edit: Further tested by building a version of our application with this patch applied, the links work now.

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

No.

Page with working link
<img width="913" alt="Screenshot 2024-03-18 at 4 45 27 PM" src="https://github.com/apache/spark/assets/5205457/dbcd1ffc-b7e6-4f84-8ca7-602c41202bf3">

Goes correctly to
<img width="539" alt="Screenshot 2024-03-18 at 4 45 36 PM" src="https://github.com/apache/spark/assets/5205457/89111c82-b24a-4b33-895f-9c0131e8acb5">

Before it would redirect and we'd get a 404.

<img width="639" alt="image" src="https://github.com/apache/spark/assets/5205457/1adfeba1-a1f6-4c35-9c39-e077c680baef">

Closes #45527 from HuwCampbell/patch-1.

Authored-by: Huw Campbell <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 9b466d3)
Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun pushed a commit that referenced this pull request Mar 18, 2024
### What changes were proposed in this pull request?

Like SPARK-24553, this PR aims to fix redirect issues (incorrect 302) when one is using proxy settings. Change the generated link to be consistent with other links and include a trailing slash

### Why are the changes needed?

When using a proxy, an invalid redirect is issued if this is not included

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

Only that people will be able to use these links if they are using a proxy

### How was this patch tested?

With a proxy installed I went to the location this link would generate and could go to the page, when it redirects with the link as it exists.

Edit: Further tested by building a version of our application with this patch applied, the links work now.

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

No.

Page with working link
<img width="913" alt="Screenshot 2024-03-18 at 4 45 27 PM" src="https://github.com/apache/spark/assets/5205457/dbcd1ffc-b7e6-4f84-8ca7-602c41202bf3">

Goes correctly to
<img width="539" alt="Screenshot 2024-03-18 at 4 45 36 PM" src="https://github.com/apache/spark/assets/5205457/89111c82-b24a-4b33-895f-9c0131e8acb5">

Before it would redirect and we'd get a 404.

<img width="639" alt="image" src="https://github.com/apache/spark/assets/5205457/1adfeba1-a1f6-4c35-9c39-e077c680baef">

Closes #45527 from HuwCampbell/patch-1.

Authored-by: Huw Campbell <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 9b466d3)
Signed-off-by: Dongjoon Hyun <[email protected]>
@dongjoon-hyun
Copy link
Member

Welcome to the Apache Spark community, @HuwCampbell .

I added you to the Apache Spark Contributor group and assigned SPARK-47434 to you.

Thank you again.

@HuwCampbell HuwCampbell deleted the patch-1 branch March 22, 2024 05:58
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