Skip to content

[SPARK-44318][BUILD] Remove useless dependencies - javax.ws.rs-api - #41340

Closed
panbingkun wants to merge 3 commits into
apache:masterfrom
panbingkun:remove_javax_ws_rs_api
Closed

[SPARK-44318][BUILD] Remove useless dependencies - javax.ws.rs-api#41340
panbingkun wants to merge 3 commits into
apache:masterfrom
panbingkun:remove_javax_ws_rs_api

Conversation

@panbingkun

@panbingkunpanbingkun commented May 27, 2023

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

The pr aims to remove useless dependencies - javax.ws.rs-api.

Why are the changes needed?

Make code and dependency concisely.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass GA.

@LuciferYang

Copy link
Copy Markdown
Contributor

@panbingkun Can you explain why these two dependencies are no longer useful?

@dongjoon-hyun
dongjoon-hyun marked this pull request as draft May 28, 2023 02:35
@panbingkunpanbingkun changed the title [WIP] Remove some dependency in pom[SPARK-44318][BUILD] Remove useless dependencies - javax.ws.rs-apiJul 6, 2023
@panbingkun

Copy link
Copy Markdown
ContributorAuthor

1.(Jersey depend on it) The first PR to introduce the dependency of javax.ws.rs.javax.ws.rs-api
PR: [SPARK-12154] Upgrade to Jersey 2
CommitID: b7fdc23

b7fdc23#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R624-R627
image

2.Because Jersey depends on it, however, with the upgrade of Jersey version, the rs-api package name it depends on has been moved from: javax.ws.rs.javax.ws.rs-api to: jakarta.ws.rs.jakarta.ws.rs-api in version 2.28, but there is no corresponding removal in Spark.

imageimage
image

4.In fact, we can only depends on [jakarta.ws.rs](https://mvnrepository.com/artifact/jakarta.ws.rs) » [jakarta.ws.rs-api](https://mvnrepository.com/artifact/jakarta.ws.rs/jakarta.ws.rs-api) in Spark because its package name has changed.

@panbingkun
panbingkun marked this pull request as ready for review July 6, 2023 07:08
@panbingkun

Copy link
Copy Markdown
ContributorAuthor

@dongjoon-hyun

Copy link
Copy Markdown
Member

There is no change on dependency manifest file?

@panbingkun

Copy link
Copy Markdown
ContributorAuthor

There is no change on dependency manifest file?

Yes, that's right, it doesn't exist in spark-deps-hadoop-3-hive-2.3.
Only the changed package exists:
image

@github-actions

Copy link
Copy Markdown

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

dongjoon-hyun pushed a commit that referenced this pull request Oct 15, 2024
### What changes were proposed in this pull request?
- To Remove the dependency of `javax.ws.rs.ws-rs-api` as it's no longer required.
Prior discussion can be found on:
- #41340
- #45154
### Why are the changes needed?
In the past, the codebase used to have a few .scala classes referencing and using the `ws-rs-api`, such as b7fdc23#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R624-R627
However as the time passed by, all usages of `ws-rs-api` are either got removed / refactored. Hence there is no need to have it import on root POM as now and we can always re-introduce it later, if the usage can be justified again.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Unit-test, to make sure the codebase is not impacted by the removal of the dependency.
### Was this patch authored or co-authored using generative AI tooling?
No
Closes#48461 from hiufung-kwok/ft-hf-SPARK-49963-remove-ws-rs-api.
Authored-by: HiuFung Kwok <hiufung.kwok.852@gmail.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@panbingkun@LuciferYang@dongjoon-hyun