Skip to content

Commit

Permalink
update volume: changing new_name to be optional and making empty upda…
Browse files Browse the repository at this point in the history
…tes change nothing (unitycatalog#254)

**PR Checklist**

- [x] A description of the changes is added to the description of this
PR.
- [x] If there is a related issue, make sure it is linked to this PR.
- [x] If you've fixed a bug or added code that should be tested, add
tests!
- [x] If you've added or modified a feature, documentation in `docs` is
updated

**Description of changes**

<!-- Please state what you've changed and how it might affect the users.
-->
Making `update volume` work similarly to how `update catalog` was
changed to work in PR unitycatalog#158 and `update schema`was changed to work in PR
unitycatalog#219

Closes unitycatalog#110

Two main changes:
- Changed the parameter new_name to be optional in the server and CLI.
- Changed both the CLI and `VolumeRepository.updateVolume` to not update
anything if no parameters are provided to update.
  - CLI just prints a help message listing all optional parameters.
  - `VolumeRepository.updateVolume` just returns existing `VolumeInfo`.
  • Loading branch information
VillePuuska authored Jul 18, 2024
1 parent 5f00e81 commit bb8dd4f
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 23 deletions.
4 changes: 3 additions & 1 deletion docs/usage/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,11 @@ bin/uc volume update --full_name <catalog>.<schema>.<volume> --new_name <new_nam
- `catalog` : The name of the catalog.
- `schema` : The name of the schema.
- `volume` : The name of the volume.
- `new_name` : The new name of the volume.
- `new_name` : *[Optional]* The new name of the volume.
- `comment` : *[Optional]* The new description of the volume.

*Note:* at least one of the optional parameters must be specified.

Example:
```sh
bin/uc volume update --full_name my_catalog.my_schema.my_volume --new_name my_updated_volume --comment "Updated Volume"
Expand Down
13 changes: 11 additions & 2 deletions examples/cli/src/main/java/io/unitycatalog/cli/VolumeCli.java
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,17 @@ private static String updateVolume(VolumesApi apiClient, JSONObject json)
throws JsonProcessingException, ApiException {
String volumeFullName = json.getString(CliParams.FULL_NAME.getServerParam());
json.remove(CliParams.FULL_NAME.getServerParam());
UpdateVolumeRequestContent updateVolumeRequest;
updateVolumeRequest = objectMapper.readValue(json.toString(), UpdateVolumeRequestContent.class);
if (json.length() == 0) {
List<CliParams> optionalParams =
CliUtils.cliOptions.get(CliUtils.VOLUME).get(CliUtils.UPDATE).getOptionalParams();
String errorMessage = "No parameters to update, please provide one of:";
for (CliParams param : optionalParams) {
errorMessage += "\n --" + param.val();
}
throw new CliException(errorMessage);
}
UpdateVolumeRequestContent updateVolumeRequest =
objectMapper.readValue(json.toString(), UpdateVolumeRequestContent.class);
return objectWriter.writeValueAsString(
apiClient.updateVolume(volumeFullName, updateVolumeRequest));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ public List<CliParams> getOptionalParams() {
put(
UPDATE,
new CliOptions(
List.of(CliParams.FULL_NAME, CliParams.NEW_NAME),
List.of(CliParams.COMMENT)));
List.of(CliParams.FULL_NAME),
List.of(CliParams.COMMENT, CliParams.NEW_NAME)));
put(DELETE, new CliOptions(List.of(CliParams.FULL_NAME), List.of()));
put(READ, new CliOptions(List.of(CliParams.FULL_NAME), List.of(CliParams.PATH)));
put(WRITE, new CliOptions(List.of(CliParams.FULL_NAME), List.of(CliParams.PATH)));
Expand Down Expand Up @@ -453,7 +453,8 @@ public static void printHelp() {
"By default, the client will connect to UC running locally at http://localhost:8080\n");
System.out.println("To connect to specific UC server, use --server https://<host>\n");
System.out.println(
"Currently, auth using bearer token is supported. Please specify the token via --auth_token <PAT Token>\n");
"Currently, auth using bearer token is supported. Please specify the token via --auth_token"
+ " <PAT Token>\n");
System.out.println(
"For detailed help on entity specific operations, use bin/uc <entity> --help");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,17 @@ public VolumeInfo getVolume(String volumeFullName) {
@Override
public VolumeInfo updateVolume(
String volumeFullName, UpdateVolumeRequestContent updateVolumeRequest) {
String[] args =
addServerAndAuthParams(
List.of(
"volume",
"update",
"--full_name",
volumeFullName,
"--new_name",
updateVolumeRequest.getNewName(),
"--comment",
updateVolumeRequest.getComment()),
config);
List<String> argsList =
new ArrayList<>(List.of("volume", "update", "--full_name", volumeFullName));
if (updateVolumeRequest.getNewName() != null) {
argsList.add("--new_name");
argsList.add(updateVolumeRequest.getNewName());
}
if (updateVolumeRequest.getComment() != null) {
argsList.add("--comment");
argsList.add(updateVolumeRequest.getComment());
}
String[] args = addServerAndAuthParams(argsList, config);
JsonNode updatedVolumeInfo = executeCLICommand(args);
return objectMapper.convertValue(updatedVolumeInfo, VolumeInfo.class);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,9 @@ private VolumeInfo convertFromDAO(
}

public VolumeInfo updateVolume(String name, UpdateVolumeRequestContent updateVolumeRequest) {
ValidationUtils.validateSqlObjectName(updateVolumeRequest.getNewName());
if (updateVolumeRequest.getNewName() != null) {
ValidationUtils.validateSqlObjectName(updateVolumeRequest.getNewName());
}
String[] namespace = name.split("\\.");
String catalog = namespace[0], schema = namespace[1], volume = namespace[2];
try (Session session = SESSION_FACTORY.openSession()) {
Expand All @@ -232,6 +234,10 @@ public VolumeInfo updateVolume(String name, UpdateVolumeRequestContent updateVol
"Volume already exists: " + updateVolumeRequest.getNewName());
}
}
if (updateVolumeRequest.getNewName() == null && updateVolumeRequest.getComment() == null) {
tx.rollback();
return convertFromDAO(volumeInfo, catalog, schema);
}
if (updateVolumeRequest.getNewName() != null) {
volumeInfo.setName(updateVolumeRequest.getNewName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,18 +88,36 @@ public void testVolumeCRUD() throws ApiException {
VolumeInfo retrievedVolumeInfo = volumeOperations.getVolume(VOLUME_FULL_NAME);
assertEquals(volumeInfo, retrievedVolumeInfo);

// Update volume
System.out.println("Testing update volume..");
// Calling update volume with nothing to update should not change anything
System.out.println("Testing updating volume with nothing to update..");
UpdateVolumeRequestContent emptyUpdateVolumeRequest = new UpdateVolumeRequestContent();
VolumeInfo emptyUpdatedVolumeInfo =
volumeOperations.updateVolume(VOLUME_FULL_NAME, emptyUpdateVolumeRequest);
VolumeInfo retrievedVolumeInfo2 = volumeOperations.getVolume(VOLUME_FULL_NAME);
assertEquals(volumeInfo, retrievedVolumeInfo2);

// Update volume name without updating comment
System.out.println("Testing update volume: changing name..");
UpdateVolumeRequestContent updateVolumeRequest =
new UpdateVolumeRequestContent().newName(VOLUME_NEW_NAME).comment(COMMENT);
// Set update details
new UpdateVolumeRequestContent().newName(VOLUME_NEW_NAME);
VolumeInfo updatedVolumeInfo =
volumeOperations.updateVolume(VOLUME_FULL_NAME, updateVolumeRequest);
assertEquals(updateVolumeRequest.getNewName(), updatedVolumeInfo.getName());
assertEquals(updateVolumeRequest.getComment(), updatedVolumeInfo.getComment());
assertEquals(VOLUME_NEW_FULL_NAME, updatedVolumeInfo.getFullName());
assertNotNull(updatedVolumeInfo.getUpdatedAt());

// Update volume comment without updating name
System.out.println("Testing update volume: changing comment..");
UpdateVolumeRequestContent updateVolumeRequest2 =
new UpdateVolumeRequestContent().comment(COMMENT);
VolumeInfo updatedVolumeInfo2 =
volumeOperations.updateVolume(VOLUME_NEW_FULL_NAME, updateVolumeRequest2);
assertEquals(VOLUME_NEW_NAME, updatedVolumeInfo2.getName());
assertEquals(updateVolumeRequest2.getComment(), updatedVolumeInfo2.getComment());
assertEquals(VOLUME_NEW_FULL_NAME, updatedVolumeInfo2.getFullName());
assertNotNull(updatedVolumeInfo2.getUpdatedAt());

// Delete volume
System.out.println("Testing delete volume..");
volumeOperations.deleteVolume(VOLUME_NEW_FULL_NAME);
Expand Down

0 comments on commit bb8dd4f

Please sign in to comment.