Add namespace parameter to getLockState

pkoenig10 commented 2 weeks ago

Before this PR

LockService.getLockState always fails when using external Timelock.

The LockRpcClient JAX-RS client interface has a class level @Path("/{namespace}/lock") annotation, but the getLockState method does not include a @PathParam("namespace") parameter. So the {namespace} template does not get replaced in the URL and Timelock sees a literal {namespace} for the namespace value, which fails the validation here:

https://github.com/palantir/atlasdb/blob/1830ac97688d5c45e3d99b321a88891de6dd4bf2/timelock-impl/src/main/java/com/palantir/atlasdb/timelock/TimelockNamespaces.java#L120-L121

com.palantir.logsafe.exceptions.SafeIllegalArgumentException: Invalid namespace
    at com.palantir.logsafe.Preconditions.checkArgument(Preconditions.java:79)
    at com.palantir.atlasdb.timelock.TimelockNamespaces.createNewClient(TimelockNamespaces.java:120)
    at com.palantir.atlasdb.timelock.TimelockNamespaces.lambda$get$0(TimelockNamespaces.java:103)
    at java.base/java.util.concurrent.ConcurrentHashMap.computeIfAbsent(ConcurrentHashMap.java:1708)
    at com.palantir.atlasdb.timelock.TimelockNamespaces.get(TimelockNamespaces.java:98)
    at com.palantir.atlasdb.timelock.LockV1Resource.getLockService(LockV1Resource.java:556)
    at com.palantir.atlasdb.timelock.LockV1Resource.getLockState(LockV1Resource.java:552)

After this PR

This PR adds the required @PathParam("namespace") parameter to getLockState. This is an ABI break, but this feels acceptable because:

This method is simply broken without it.

At least, in places using external Timelock, which is nearly all production environments.
There are no implementations or callers in internal repos.

This PR does not change the LockService API, only the internal client interface.

I've confirmed with local testing that this now works when using external Timelock.

changelog-app[bot] commented 2 weeks ago

Generate changelog in changelog-dir>`changelog/@unreleased`</changelog-dir

What do the change types mean?

- `feature`: A new feature of the service. - `improvement`: An incremental improvement in the functionality or operation of the service. - `fix`: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way. - `break`: Has the potential to break consumers of this service's API, inclusive of both Palantir services and external consumers of the service's API (e.g. customer-written software or integrations). - `deprecation`: Advertises the intention to remove service functionality without any change to the operation of the service itself. - `manualTask`: Requires the possibility of manual intervention (running a script, eyeballing configuration, performing database surgery, ...) at the time of upgrade for it to succeed. - `migration`: A fully automatic upgrade migration task with no engineer input required. _Note: only one type should be chosen._

How are new versions calculated?

- ❗The `break` and `manual task` changelog types will result in a major release! - 🐛 The `fix` changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease. - ✨ All others will result in a minor version release.

Type

- [ ] Feature - [ ] Improvement - [x] Fix - [ ] Break - [ ] Deprecation - [ ] Manual task - [ ] Migration

Description

`LockService.getLockState` now works with external Timelock.

**Check the box to generate changelog(s)** - [x] Generate changelog entry

svc-autorelease commented 1 week ago

Failed to push a commit onto develop to move @unreleased changelogs

com.palantir.conjure.java.api.errors.UnknownRemoteException: Response status: 502
    at com.palantir.conjure.java.dialogue.serde.ErrorDecoder.decodeInternal(ErrorDecoder.java:123)
    at com.palantir.conjure.java.dialogue.serde.ErrorDecoder.decode(ErrorDecoder.java:68)
    at com.palantir.conjure.java.dialogue.serde.ConjureBodySerDe$EmptyBodyDeserializer.deserialize(ConjureBodySerDe.java:351)
    at com.palantir.conjure.java.dialogue.serde.ConjureBodySerDe$EmptyBodyDeserializer.deserialize(ConjureBodySerDe.java:338)
    at com.palantir.conjure.java.client.jaxrs.DialogueFeignClient$RemoteExceptionDecoder.decode(DialogueFeignClient.java:336)
    at feign.SynchronousMethodHandler.executeAndDecode(SynchronousMethodHandler.java:134)
    at feign.SynchronousMethodHandler.invoke(SynchronousMethodHandler.java:76)
    at feign.ReflectiveFeign$FeignInvocationHandler.invoke(ReflectiveFeign.java:103)
    at jdk.proxy2/jdk.proxy2.$Proxy83.createTree(Unknown Source)
    at jdk.internal.reflect.GeneratedMethodAccessor244.invoke(Unknown Source)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at com.palantir.github.clients.PerTraceRequestCountingClientFactory.invoke(PerTraceRequestCountingClientFactory.java:35)
    at com.palantir.github.clients.PerTraceRequestCountingClientFactory.lambda$client$0(PerTraceRequestCountingClientFactory.java:29)
    at jdk.proxy2/jdk.proxy2.$Proxy83.createTree(Unknown Source)
    at jdk.internal.reflect.GeneratedMethodAccessor244.invoke(Unknown Source)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at com.palantir.github.clients.ErrorTrackingExternalClientFactory.invoke(ErrorTrackingExternalClientFactory.java:36)
    at com.palantir.github.clients.ErrorTrackingExternalClientFactory.lambda$client$0(ErrorTrackingExternalClientFactory.java:30)
    at jdk.proxy2/jdk.proxy2.$Proxy83.createTree(Unknown Source)
    at com.palantir.autorelease.CommitFactory.createCommit(CommitFactory.java:100)
    at com.palantir.autorelease.CommitFactory.createCommit(CommitFactory.java:70)
    at com.palantir.autorelease.ReleaserHelper.moveChangelogs(ReleaserHelper.java:64)
    at com.palantir.autorelease.Releaser.labelRelease(Releaser.java:229)
    at com.palantir.autorelease.DefaultRepositoryArchetype.automatedRelease(DefaultRepositoryArchetype.java:117)
    at com.palantir.autorelease.Repository.automatedRelease(Repository.java:63)
    at com.palantir.autorelease.label.DefaultWebhookHandler.handlePullRequestClosedEvent(DefaultWebhookHandler.java:224)
    at com.palantir.autorelease.label.DefaultWebhookHandler.handleWebhook(DefaultWebhookHandler.java:128)
    at com.palantir.github.GithubWebhookResource.lambda$receiveWebhook$0(GithubWebhookResource.java:58)
    at com.palantir.tracing.Tracers$TracingAwareRunnable.run(Tracers.java:584)
    at com.palantir.tritium.metrics.TaggedMetricsExecutorService$TaggedMetricsRunnable.run(TaggedMetricsExecutorService.java:142)
    at com.palantir.nylon.threads.RenamingExecutorService$RenamingRunnable.run(RenamingExecutorService.java:92)
    at org.jboss.threads.EnhancedViewExecutor$EnhancedViewExecutorRunnable.run(EnhancedViewExecutor.java:519)
    at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
    at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2513)
    at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1538)
    at com.palantir.tritium.metrics.TaggedMetricsThreadFactory$InstrumentedTask.run(TaggedMetricsThreadFactory.java:69)
    at java.base/java.lang.Thread.run(Thread.java:840)
    Suppressed: com.palantir.conjure.java.dialogue.serde.ErrorDecoder$ResponseDiagnostic: Response Diagnostic Information: {status=502, Server=GitHub.com, Content-Type=application/json, Content-Length=32, Date=Mon, 24 Jun 2024 08:54:30 GMT}
    Suppressed: com.palantir.logsafe.exceptions.SafeRuntimeException: unknown error: {serviceName=github, errorBody={
  "message": "Server Error"
}
}
        at com.palantir.github.clients.ErrorLoggingErrorTracker.trackUnknownRemoteError(ErrorLoggingErrorTracker.java:23)
        at com.palantir.github.clients.ErrorTracker.lambda$andAlso$0(ErrorTracker.java:14)
        at com.palantir.github.clients.ErrorTrackingExternalClientFactory.invoke(ErrorTrackingExternalClientFactory.java:41)
        ... 20 more

palantir / atlasdb