Skip to content

Commit

Permalink
Store self serve replication API destination input as uppercase (#260)
Browse files Browse the repository at this point in the history
## Summary

<!--- HINT: Replace #nnn with corresponding Issue number, if you are
fixing an existing issue -->
This PR saves the destination input for the self serve replication API
as uppercase in the server side to maintain consistency across inputs.
The API still accepts both lower/upper case inputs, but the server
processes it as uppercase.

## Changes

- [ ] Client-facing API Changes
- [ ] Internal API Changes
- [ ] Bug Fixes
- [ ] New Features
- [ ] Performance Improvements
- [ ] Code Style
- [x] Refactoring
- [ ] Documentation
- [ ] Tests

For all the boxes checked, please include additional details of the
changes made in this pull request.

## Testing Done
<!--- Check any relevant boxes with "x" -->

- [x] Manually Tested on local docker setup. Please include commands
ran, and their output.
- [ ] Added new tests for the changes made.
- [x] Updated existing tests to reflect the changes made.
- [ ] No tests added or updated. Please explain why. If unsure, please
feel free to ask for help.
- [ ] Some other form of testing like staging or soak time in
production. Please explain.

For all the boxes checked, include a detailed description of the testing
done for the changes made in this pull request.

Updated unit testing and tested on local docker:
```
spark.sql("alter table u_tableowner.tbl1 set policy (replication=({destination:'bb', interval:12H}))")
```
in table properties:
```
  "replication": {
    "config": [
      {
        "destination": "\u0027BB\u0027",
        "interval": "12H",
      }
    ]
```
```
spark.sql("alter table u_tableowner.tbl1 set policy (replication=({destination:'BB', interval:12H}))")
```
in table properties:
```
  "replication": {
    "config": [
      {
        "destination": "\u0027BB\u0027",
        "interval": "12H",
      }

```


# Additional Information

- [ ] Breaking Changes
- [ ] Deprecations
- [ ] Large PR broken into smaller PRs, and PR plan linked in the
description.

For all the boxes checked, include additional details of the changes
made in this pull request.
  • Loading branch information
chenselena authored Dec 6, 2024
1 parent e6299ec commit 6a54b02
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import com.linkedin.openhouse.tables.utils.IntervalToCronConverter;
import java.util.List;
import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
import org.mapstruct.Mapper;
import org.mapstruct.Named;

Expand Down Expand Up @@ -119,24 +120,25 @@ private Replication mapReplicationPolicies(Replication replicationPolicy) {
replicationPolicy.getConfig().stream()
.map(
replication -> {
if (replication.getInterval() == null || replication.getInterval().isEmpty()) {
return replication
.toBuilder()
.interval(ReplicationInterval.DEFAULT.getInterval())
.cronSchedule(
IntervalToCronConverter.generateCronExpression(
ReplicationInterval.DEFAULT.getInterval()))
.build();
if (replication == null || StringUtils.isEmpty(replication.getDestination())) {
return null;
}
if (replication.getCronSchedule() == null) {
return replication
.toBuilder()
.cronSchedule(
IntervalToCronConverter.generateCronExpression(
replication.getInterval()))
.build();
String destination = replication.getDestination().toUpperCase();
String interval = replication.getInterval();
String cronSchedule = replication.getCronSchedule();

if (StringUtils.isEmpty(interval)) {
interval = ReplicationInterval.DEFAULT.getInterval();
}
if (StringUtils.isEmpty(cronSchedule)) {
cronSchedule = IntervalToCronConverter.generateCronExpression(interval);
}
return replication;
return replication
.toBuilder()
.destination(destination)
.interval(interval)
.cronSchedule(cronSchedule)
.build();
})
.collect(Collectors.toList());
return replicationPolicy.toBuilder().config(replicationConfig).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,7 @@ public void testUpdateSucceedsForReplicationConfig() throws Exception {
JsonPath.read(
mvcResult.getResponse().getContentAsString(), "$.policies.replication.config[0]");

Assertions.assertEquals(updatedReplication.get("destination"), "clusterA");
Assertions.assertEquals(updatedReplication.get("destination"), "CLUSTERA");
Assertions.assertEquals(updatedReplication.get("interval"), "12H");
Assertions.assertTrue(
RequestAndValidateHelper.validateCronSchedule(updatedReplication.get("cronSchedule")));
Expand Down Expand Up @@ -1128,7 +1128,7 @@ public void testUpdateSucceedsForReplicationAndRetention() throws Exception {
JsonPath.read(
mvcResult.getResponse().getContentAsString(), "$.policies.replication.config[0]");

Assertions.assertEquals(updatedReplication.get("destination"), "clusterA");
Assertions.assertEquals(updatedReplication.get("destination"), "CLUSTERA");
Assertions.assertEquals(updatedReplication.get("interval"), "1D");
Assertions.assertTrue(
RequestAndValidateHelper.validateCronSchedule(updatedReplication.get("cronSchedule")));
Expand Down Expand Up @@ -1178,15 +1178,15 @@ public void testUpdateSucceedsForMultipleReplicationConfig() throws Exception {
JsonPath.read(
mvcResult.getResponse().getContentAsString(), "$.policies.replication.config[0]");

Assertions.assertEquals(updatedReplication.get("destination"), "clusterA");
Assertions.assertEquals(updatedReplication.get("destination"), "CLUSTERA");
Assertions.assertEquals(updatedReplication.get("interval"), "1D");
Assertions.assertTrue(
RequestAndValidateHelper.validateCronSchedule(updatedReplication.get("cronSchedule")));
updatedReplication =
JsonPath.read(
mvcResult.getResponse().getContentAsString(), "$.policies.replication.config[1]");

Assertions.assertEquals(updatedReplication.get("destination"), "clusterB");
Assertions.assertEquals(updatedReplication.get("destination"), "CLUSTERB");
Assertions.assertEquals(updatedReplication.get("interval"), "12H");
Assertions.assertTrue(
RequestAndValidateHelper.validateCronSchedule(updatedReplication.get("cronSchedule")));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public final class TableModelConstants {
RETENTION_POLICY =
Retention.builder().count(3).granularity(TimePartitionSpec.Granularity.HOUR).build();
ArrayList<ReplicationConfig> configs = new ArrayList<>();
configs.add(ReplicationConfig.builder().destination("cluster1").interval("12H").build());
configs.add(ReplicationConfig.builder().destination("CLUSTER1").interval("12H").build());
REPLICATION_POLICY = Replication.builder().config(configs).build();

RETENTION_POLICY_WITH_PATTERN =
Expand Down

0 comments on commit 6a54b02

Please sign in to comment.