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.

Sign up for GitHub

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

Jump to bottom

quarkus-oidc brings jackson #19688

Closed
XakepSDK opened this issue Aug 26, 2021 · 5 comments · Fixed by #19689
Closed

quarkus-oidc brings jackson #19688

XakepSDK opened this issue Aug 26, 2021 · 5 comments · Fixed by #19689
Labels
area/jackson Issues related to Jackson (JSON library) area/oidc kind/enhancement New feature or request
Milestone
2.3.0.CR1

Comments

@XakepSDK
Copy link

XakepSDK commented Aug 26, 2021 •
edited
Loading

Describe the bug

quarkus-oidc extension brings jackson dependency. I checked code and looks like it doesn't depend on jackson, at least directly.

Expected behavior

oidc doesn't bring jackson

Actual behavior

oidc brings jackson

How to Reproduce?

https://code.quarkus.io/

  1. Add openid connect extension
  2. Check dependencies graph
@XakepSDK XakepSDK added the kind/bug Something isn't working label Aug 26, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Aug 26, 2021

/cc @geoand, @gsmet, @pedroigor, @sberyozkin

@quarkus-bot quarkus-bot bot added area/jackson Issues related to Jackson (JSON library) area/oidc labels Aug 26, 2021
@geoand
Copy link
Contributor

geoand commented Aug 26, 2021 •
edited
Loading

The dependency on Jackson was added in #5470.

With Quarkus 2.x which includes Vert.x 4.x which in turn has depends more loosely on Jackson (and contains the proper GraalvM substitutions when Jackson Databind is not present), perhaps that fix is no longer needed.

I ran the oidc-code-flow and oidc integration tests without Jackson and they seemed to work.

@sberyozkin I opened a draft PR to see how things go in my fork.

geoand added a commit to geoand/quarkus that referenced this issue Aug 26, 2021
@geoand
Remove unnecessary Jackson dependency from oidc extension
922837d 
See quarkusio#19688 (comment)
for the reasoning behind this

Fixes: quarkusio#19688
@geoand geoand mentioned this issue Aug 26, 2021
Merged
@geoand geoand added kind/enhancement New feature or request and removed kind/bug Something isn't working labels Aug 26, 2021
@sberyozkin
Copy link
Member

I'm not sure - there is also #14796, for ex, Vert.x JsonObject needs it I believe

@geoand
Copy link
Contributor

geoand commented Aug 26, 2021

CI should cover that case as well, right?

@sberyozkin
Copy link
Member

Hope so, but for ex #14796, NPE was certainly not available initially, I don't recall how it appeared

geoand added a commit to geoand/quarkus that referenced this issue Aug 26, 2021
@geoand
Remove unnecessary Jackson dependency from oidc extension
810c140 
See quarkusio#19688 (comment)
for the reasoning behind this

Fixes: quarkusio#19688
sberyozkin added a commit that referenced this issue Aug 27, 2021
@sberyozkin
Merge pull request #19689 from geoand/#19688
ed2d00a 
Remove unnecessary Jackson dependency from oidc extension
@quarkus-bot quarkus-bot bot added this to the 2.3 - main milestone Aug 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/jackson Issues related to Jackson (JSON library) area/oidc kind/enhancement New feature or request
Projects
None yet
Milestone
2.3.0.CR1
Development

Successfully merging a pull request may close this issue.

Remove unnecessary Jackson dependency from oidc extension geoand/quarkus
3 participants
@sberyozkin @geoand @XakepSDK