-
Notifications
You must be signed in to change notification settings - Fork 15
Qos Service config #2644
Qos Service config #2644
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,8 +17,15 @@ | |
package com.palantir.atlasdb.qos; | ||
|
||
public class QosServiceResource implements QosService { | ||
|
||
private QosServiceRuntimeConfig config; | ||
|
||
public QosServiceResource(QosServiceRuntimeConfig config) { | ||
this.config = config; | ||
} | ||
|
||
@Override | ||
public int getLimit(String client) { | ||
return Integer.MAX_VALUE; | ||
public long getLimit(String client) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason for int->long? just generally giving ourselves more room? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exactly, just to give us more room. |
||
return config.clientLimits().get(client); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
/* | ||
* Copyright 2017 Palantir Technologies, Inc. All rights reserved. | ||
* | ||
* Licensed under the BSD-3 License (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://opensource.org/licenses/BSD-3-Clause | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package com.palantir.atlasdb.qos; | ||
|
||
import java.util.Map; | ||
|
||
import org.immutables.value.Value; | ||
|
||
import com.fasterxml.jackson.databind.annotation.JsonDeserialize; | ||
import com.fasterxml.jackson.databind.annotation.JsonSerialize; | ||
|
||
@JsonDeserialize(as = ImmutableQosServiceRuntimeConfig.class) | ||
@JsonSerialize(as = ImmutableQosServiceRuntimeConfig.class) | ||
@Value.Immutable | ||
public abstract class QosServiceRuntimeConfig { | ||
abstract Map<String, Long> clientLimits(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
/* | ||
* Copyright 2017 Palantir Technologies, Inc. All rights reserved. | ||
* | ||
* Licensed under the BSD-3 License (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://opensource.org/licenses/BSD-3-Clause | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package com.palantir.atlasdb.qos; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
import static org.assertj.core.api.Assertions.entry; | ||
|
||
import java.io.File; | ||
import java.io.IOException; | ||
|
||
import org.junit.Test; | ||
|
||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; | ||
import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator; | ||
|
||
public class QosRuntimeConfigDeserializationTest { | ||
|
||
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(new YAMLFactory() | ||
.disable(YAMLGenerator.Feature.USE_NATIVE_TYPE_ID) | ||
.disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER)); | ||
|
||
@Test | ||
public void canDeserializeQosServerConfiguration() throws IOException { | ||
File testConfigFile = new File(QosServiceRuntimeConfig.class.getResource("/qos.yml").getPath()); | ||
QosServiceRuntimeConfig configuration = OBJECT_MAPPER.readValue(testConfigFile, QosServiceRuntimeConfig.class); | ||
|
||
assertThat(configuration.clientLimits()).hasSize(2); | ||
|
||
assertThat(configuration.clientLimits()).containsOnly(entry("test", 10L), entry("test2", 20L)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: could construct a config object here and assert equals |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
/* | ||
* Copyright 2017 Palantir Technologies, Inc. All rights reserved. | ||
* | ||
* Licensed under the BSD-3 License (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://opensource.org/licenses/BSD-3-Clause | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package com.palantir.atlasdb.qos; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
import java.util.HashMap; | ||
import java.util.Map; | ||
|
||
import org.junit.Test; | ||
|
||
public class QosServiceRuntimeConfigTest { | ||
@Test | ||
public void canBuildFromEmptyClientLimits() { | ||
assertThat(ImmutableQosServiceRuntimeConfig.builder().clientLimits(new HashMap<>()).build()) | ||
.isInstanceOf(QosServiceRuntimeConfig.class); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: are we just checking that it doesn't throw? if so no need for the class check |
||
} | ||
|
||
@Test | ||
public void canBuildFromSingleClientLimit() { | ||
Map<String, Long> clientLimits = new HashMap<>(1); | ||
clientLimits.put("test_client", 10L); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: can use |
||
assertThat(ImmutableQosServiceRuntimeConfig.builder().clientLimits(clientLimits).build()) | ||
.isInstanceOf(QosServiceRuntimeConfig.class); | ||
} | ||
|
||
@Test | ||
public void canBuildFromMultipleClientLimits() { | ||
Map<String, Long> clientLimits = new HashMap<>(1); | ||
clientLimits.put("test_client", 10L); | ||
clientLimits.put("test_client2", 100L); | ||
assertThat(ImmutableQosServiceRuntimeConfig.builder().clientLimits(clientLimits).build()) | ||
.isInstanceOf(QosServiceRuntimeConfig.class); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
clientLimits: | ||
test: 10 | ||
test2: 20 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,7 +52,6 @@ public void initialize(Bootstrap<TimeLockServerConfiguration> bootstrap) { | |
public void run(TimeLockServerConfiguration configuration, Environment environment) { | ||
environment.getObjectMapper().registerModule(new Jdk8Module()); | ||
environment.jersey().register(HttpRemotingJerseyFeature.INSTANCE); | ||
environment.jersey().register(new QosServiceResource()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this will now be covered by internal tooling stuff, I guess? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes and also the |
||
|
||
CombinedTimeLockServerConfiguration combined = TimeLockConfigMigrator.convert(configuration, environment); | ||
Consumer<Object> registrar = component -> environment.jersey().register(component); | ||
|
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.
Is this really a compile dep for API?
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.
Was thinking about adding the
@Size(min = 1)
for clientLimits, but decided to go without it as the service will not be able to startup without configuring it if I add the check.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.
removed the stray dependency.