-
Notifications
You must be signed in to change notification settings - Fork 110
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
update gitlab4j@5.1.0 and use Long for Gitlab entities' IDs #414
base: master
Are you sure you want to change the base?
Conversation
a44bf9f
to
d69c7b8
Compare
@kevin-m-knight-gs Seems like we're blocked on this one since See their code here: They make the call |
5d608e8
to
950da5a
Compare
@kevin-m-knight-gs I think I found a way to make this work. Please checkout the explanation stated in the PR description for my approach. I don't think a lot people have complained about this, but I start seeing more and more on my pipelines on https://legend.finos.org/sdlc/api/projects/UAT-27078619/workspaces/Tarta/workflows/570831036/jobs
You can see the job ID is |
legend-sdlc-server/src/main/java/org/finos/legend/sdlc/server/gitlab/GitLabProjectId.java
Outdated
Show resolved
Hide resolved
legend-sdlc-server/src/main/java/org/finos/legend/sdlc/server/gitlab/api/BaseGitLabApi.java
Show resolved
Hide resolved
legend-sdlc-server/src/test/java/org/finos/legend/sdlc/server/gitlab/TestGitLabProjectId.java
Outdated
Show resolved
Hide resolved
2d3e993
to
63ea663
Compare
5e86942
to
563a8d2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor changes
legend-sdlc-server/src/test/java/org/finos/legend/sdlc/server/gitlab/TestGitLabProjectId.java
Outdated
Show resolved
Hide resolved
...d-sdlc-server/src/test/java/org/finos/legend/sdlc/server/resources/SDLCServerClientRule.java
Outdated
Show resolved
Hide resolved
...d-sdlc-server/src/test/java/org/finos/legend/sdlc/server/resources/SDLCServerClientRule.java
Outdated
Show resolved
Hide resolved
...d-sdlc-server/src/test/java/org/finos/legend/sdlc/server/resources/SDLCServerClientRule.java
Outdated
Show resolved
Hide resolved
...r/src/test/java/org/finos/legend/sdlc/server/LegendSDLCServerForTestJacksonJsonProvider.java
Outdated
Show resolved
Hide resolved
...-server-shared/src/main/java/org/finos/legend/sdlc/server/BaseServerJacksonJsonProvider.java
Outdated
Show resolved
Hide resolved
...-server-shared/src/main/java/org/finos/legend/sdlc/server/BaseServerJacksonJsonProvider.java
Outdated
Show resolved
Hide resolved
...r/src/test/java/org/finos/legend/sdlc/server/LegendSDLCServerForTestJacksonJsonProvider.java
Outdated
Show resolved
Hide resolved
f3bb250
to
3d2af3e
Compare
I'm starting to see failing calls as pipeline job ID exceeds
INTEGER.MAX_VALUE
, so we have to useLong
instead ofInteger
for IDs, this change scope includesproject
,version
,workflow
,workflow job
This will require us to update to the latest version of
gitlab4j-api
5.0.1
as it has converted all of itsPOJO
to uselong
for idsIssue
I'm seeing this error while using
Legend
at https://legend.finos.org/sdlc (pointing at https://gitlab.com):https://legend.finos.org/sdlc/api/projects/UAT-27078619/workspaces/Tarta/workflows/570831036/jobs
You can see the job ID is
2628618079
from https://gitlab.com/finosfoundation/legend/showcase/demo-project/-/jobs/2628618079UPDATED: This seems to be more complicated than I thought.
First,
gitlab4j-api >=14.16.0
relies onorg.glassfish.jersey.jackson.internal.jackson.jaxrs.json.JacksonJaxbJsonProvider
which is only available injersey > 2.26
. Whereas, we're usingcom.fasterxml.jackson.jaxrs.json.JacksonJaxbJsonProvider
(part of our jersey version2.25.1
). This means inlegend-sdlc-server
, I have to includeorg.glassfish.jersey.media:jersey-media-json-jackson:2.35
as a runtime dependency. In theory, we could have just upgraded all of our Jersey dependencies to>2.26
, however, this will clash withdropwizard@1.3
(see this (issue)[https://github.com/dropwizard/dropwizard/issues/2148]) and thus, requires us to upgrade toDropwizard 2
first (see @ivan-kyosev-gs PR).However, this poses another problem, due to the
auto-discoverable
feature mechanism ofjersey
, our Jersey client/server accidentally registers the newer Jackson provider. This is problematic, especially in test, because we only register/bind themixin
classes (InMemoryWorkspace -> Workspace
, etc.) to the older provider. As such, we need to somehow exclude the newer provider leaving only the older one to be picked up byJackson
at runtime. Note that we only need to do this forLegendSDLC
server,Gitlab client
used bygitlab4j-api
will still function normally.To do this, we can explicitly set
LegendSDLC
Jersey client/server flagCommonProperties.FEATURE_AUTO_DISCOVERY_DISABLE = true
(See this). This sounds risky on paper, because it will also exclude any other things that are meant to be registered withAutoDiscoverable
interface.I have taken the time to find the list of
AutoDiscoverable
by debugging theServiceLoader
forJersey
at runtime and seems like onlyorg.glassfish.jersey.jackson.internal.JacksonAutoDiscoverable
(coming fromorg.glassfish.jersey.media:jersey-media-json-jackson:2.35
) are included. As such, I consider this is safe enough for us to use as a workaround till #371 happen and we could go the normal route of upgradingJersey
.