Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate from javax namespace to jakarta. #267

Merged
merged 1 commit into from
Sep 20, 2021

Conversation

ckipp01
Copy link

@ckipp01 ckipp01 commented Sep 20, 2021

A while back there was a migration effort on the javax -> jakarta
namespace in the jaxrs-api project. You can see this mentioned here:
https://github.com/eclipse-ee4j/jaxrs-api/releases (note that 3.0 should
have the release notes form 3.0.0 Milestone 1 it was just never
"released" on GitHub). Since the package names have changed there has
also been support on the OpenAPI side to support the Jakarta namspace as
well that started with 2.1.7. You can see this note here on
swagger-core:
https://github.com/swagger-api/swagger-core/wiki/Swagger-2.X---Getting-started#note-jakarta-namespace-support-since-version-217.
This PR follows up these changes and migrates from the javax namespace
to jakarta. While this is a breaking change, this needs to be done
sooner or later since downstream issues are starting to occur. For
example where I work we have a large amount of projects that recently
started bringing in jakarta stuff instead of the javax, which causes all
sorts of issues like:

[info]   java.lang.NoClassDefFoundError: javax/ws/rs/Path
[info]   at io.swagger.v3.jaxrs2.Reader.read(Reader.java:270)
[info]   at io.swagger.v3.jaxrs2.Reader.read(Reader.java:183)
[info]   at com.github.swagger.akka.SwaggerGenerator.filteredSwagger(SwaggerHttpService.scala:117)
[info]   at com.github.swagger.akka.SwaggerGenerator.filteredSwagger$(SwaggerHttpService.scala:116)
...

Since some transitive deps are bringing in jakarta and others javax. We
were able to eliminate these issues by ensuring that this repo also
switched over to jakarta. I think this is a good move to make to ensure
that the ecosystem continues to move forward and align on the jakarta
name change.

A while back there was a migration effort on the javax -> jakarta
namespace in the `jaxrs-api` project. You can see this mentioned here:
https://github.com/eclipse-ee4j/jaxrs-api/releases (note that 3.0 should
have the release notes form 3.0.0 Milestone 1 it was just never
"released" on GitHub). Since the package names have changed there has
also been support on the OpenAPI side to support the Jakarta namspace as
well that started with 2.1.7. You can see this note here on
`swagger-core`:
https://github.com/swagger-api/swagger-core/wiki/Swagger-2.X---Getting-started#note-jakarta-namespace-support-since-version-217.
This PR follows up these changes and migrates from the javax namespace
to jakarta. While this is a breaking change, this needs to be done
sooner or later since downstream issues are starting to occur. For
example where I work we have a large amount of projects that recently
started bringing in jakarta stuff instead of the javax, which causes all
sorts of issues like:

```
[info]   java.lang.NoClassDefFoundError: javax/ws/rs/Path
[info]   at io.swagger.v3.jaxrs2.Reader.read(Reader.java:270)
[info]   at io.swagger.v3.jaxrs2.Reader.read(Reader.java:183)
[info]   at com.github.swagger.akka.SwaggerGenerator.filteredSwagger(SwaggerHttpService.scala:117)
[info]   at com.github.swagger.akka.SwaggerGenerator.filteredSwagger$(SwaggerHttpService.scala:116)
...
```

Since some transitive deps are bringing in jakarta and others javax. We
were able to eliminate these issues by ensuring that this repo also
switched over to jakarta. I think this is a good move to make to ensure
that the ecosystem continues to move forward and align on the jakarta
name change.
@pjfanning
Copy link
Collaborator

I'm not sure about this - I might need to maintain separate branches - one for javax and one for jakarta.

https://github.com/pjfanning/swagger-akka-http-sample still works so the use of jakarta artifacts seems to be a user choice as opposed to something that everyone has to react to.

@ckipp01
Copy link
Author

ckipp01 commented Sep 20, 2021

https://github.com/pjfanning/swagger-akka-http-sample still works so the use of jakarta artifacts seems to be a user choice as opposed to something that everyone has to react to.

As far as I understand it, not really. There are no longer javax artifacts being released upstream now that everything has been migrated to jakarta. So as new versions continue to come out, they will only be jakarta. Sooner or later I believe everyone will have to migrate to jakarta.

EDIT:

Also looking at the linked repo here:

https://github.com/pjfanning/swagger-akka-http-sample/blob/bc70f41f82a69c4a38ffecdc9b89c21949346af0/build.sbt#L13

As an example, this is outdated as the newest version is actually 3.0.0, but under jakarta. So my guess is that if you bump this to use jakarta, the latest, it will break.

@pjfanning pjfanning merged commit df2f683 into swagger-akka-http:main Sep 20, 2021
@pjfanning
Copy link
Collaborator

Thanks @ckipp01 - I'll try to release a 2.6.0 release in coming days - 2.5.x can be kept for javax releases

@ckipp01
Copy link
Author

ckipp01 commented Sep 20, 2021

Thanks @ckipp01 - I'll try to release a 2.6.0 release in coming days - 2.5.x can be kept for javax releases

Sounds great. One other question, are you open to adding back in support for 2.12? Looking at the commit that removed it, it wasn't clear what the benefit of removing it was. Where we use this (I'm sure it's the same for others as well), we do some interfacing with Spark, which limits us to use 2.12 in places. Are you open to a PR adding support for 2.12 back into the matrix?

@ckipp01 ckipp01 deleted the jakarta branch September 20, 2021 10:28
@pjfanning
Copy link
Collaborator

pjfanning commented Sep 20, 2021

@ckipp01 if you can fix the 2.12 build in a PR, I will merge it - com.typesafe.akka:akka-actor_2.12:2.6.16 depends on scala-java8-compat 0.8.0 instead of 1.0.0 like scala 2.13 build does and I didn't want to have to add conditional logic about dependencies

@pjfanning pjfanning mentioned this pull request Sep 20, 2021
@pjfanning
Copy link
Collaborator

pjfanning commented Oct 1, 2021

@ckipp01 I updated https://github.com/pjfanning/swagger-akka-http-sample to use jakarta. One observation is that swagger-*-jakarta jars have the same package names as the non-jakarta versions and it can be easy to accidentally end up with both jars on the classpath - this can cause runtime problems as the wrong copy of the swagger class can be picked (getting the javax copy of the swagger class when your code needs the jakarta copy).

@ckipp01
Copy link
Author

ckipp01 commented Oct 1, 2021

@ckipp01 I updated https://github.com/pjfanning/swagger-akka-http-sample to use jakarta. One observation is that swagger-*-jakarta jars have the same package names as the non-jakarta versions and it can be easy to accidentally end up with both jars on the classpath - this can cause runtime problems as the wrong copy of the swagger class can be picked (getting the javax copy of the swagger class when your code needs the jakarta copy).

Huh, that's a bummer. 🤔 I'll try to think if there is a good way around this. The sooner everything moves to jakarta the better imo. This is a bit confusing to have both like this floating around.

Sign up for free to 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.

None yet

2 participants