Skip to content
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

includeParentChildInfo in get #604

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,36 @@ default TableDto getTable(
Boolean includeInfoDetails
) {
return getTable(catalogName, databaseName, tableName, includeInfo,
includeDefinitionMetadata, includeDataMetadata, includeInfoDetails, false);
includeDefinitionMetadata, includeDataMetadata, includeInfoDetails, false,
false);
}

/**
* Get the table.
* Add this method to maintain backward compatability with dependent services which currently uses metacatClient
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit add space after this line

* @param catalogName catalog name
* @param databaseName database name
* @param tableName table name.
* @param includeInfo true if the details need to be included
* @param includeDefinitionMetadata true if the definition metadata to be included
* @param includeDataMetadata true if the data metadata to be included
* @param includeInfoDetails true if the more info details to be included
* @param includeMetadataLocationOnly true if includeMetadataLocationOnly
* @return table
*/
default TableDto getTable(
String catalogName,
String databaseName,
String tableName,
Boolean includeInfo,
Boolean includeDefinitionMetadata,
Boolean includeDataMetadata,
Boolean includeInfoDetails,
Boolean includeMetadataLocationOnly
) {
return getTable(catalogName, databaseName, tableName, includeInfo,
includeDefinitionMetadata, includeDataMetadata, includeInfoDetails, includeMetadataLocationOnly,
false);
}

/**
Expand All @@ -306,6 +335,7 @@ default TableDto getTable(
* @param includeInfoDetails true if the more info details to be included
* @param includeMetadataLocationOnly true if only metadata location needs to be included.
* All other flags are ignored if this is set to true.
* @param includeParentChildInfo true if includeParentChildInfo
* @return table
*/
@GET
Expand Down Expand Up @@ -333,7 +363,10 @@ TableDto getTable(
Boolean includeInfoDetails,
@DefaultValue("false")
@QueryParam("includeMetadataLocationOnly")
Boolean includeMetadataLocationOnly
Boolean includeMetadataLocationOnly,
@DefaultValue("false")
@QueryParam("includeParentChildInfo")
Boolean includeParentChildInfo
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit indent

);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ default TableDto getTable(
final boolean includeInfoDetails
) {
return getTable(catalogName, databaseName, tableName, includeInfo, includeDefinitionMetadata,
includeDataMetadata, includeInfoDetails, false);
includeDataMetadata, includeInfoDetails, false, false);
}

/**
Expand All @@ -96,6 +96,7 @@ default TableDto getTable(
* @param includeInfoDetails true if the more info details to be included
* @param includeMetadataLocationOnly true if only metadata location needs to be included.
* All other flags are ignored
* @param includeParentChildInfo true if includeParentChildInfo needs to be included.
* @return table
*/
TableDto getTable(
Expand All @@ -106,7 +107,8 @@ TableDto getTable(
final boolean includeDefinitionMetadata,
final boolean includeDataMetadata,
final boolean includeInfoDetails,
final boolean includeMetadataLocationOnly
final boolean includeMetadataLocationOnly,
final boolean includeParentChildInfo
);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ services:
-Dmetacat.table.update.noUpdateOnTags=iceberg_migration_do_not_modify
-Dmetacat.event.updateIcebergTablePostEventEnabled=true
-Dmetacat.parentChildRelationshipProperties.createEnabled=true
-Dmetacat.parentChildRelationshipProperties.getEnabled=true
-Dmetacat.parentChildRelationshipProperties.renameEnabled=true
-Dmetacat.parentChildRelationshipProperties.dropEnabled=true'
labels:
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,10 @@ public TableDto getTable(
@ApiParam(value = "Whether to include only the metadata location in the response")
@RequestParam(
name = "includeMetadataLocationOnly",
defaultValue = "false") final boolean includeMetadataLocationOnly
defaultValue = "false") final boolean includeMetadataLocationOnly,
@RequestParam(
name = "includeParentChildInfo",
defaultValue = "false") final boolean includeParentChildInfo
) {
final Supplier<QualifiedName> qualifiedNameSupplier =
() -> QualifiedName.ofTable(catalogName, databaseName, tableName);
Expand All @@ -669,6 +672,7 @@ public TableDto getTable(
.put("includeDataMetadata", String.valueOf(includeDataMetadata))
.put("includeMetadataFromConnector", String.valueOf(includeInfoDetails))
.put("includeMetadataLocationOnly", String.valueOf(includeMetadataLocationOnly))
.put("includeParentChildInfo", String.valueOf(includeParentChildInfo))
.build(),
() -> {
final Optional<TableDto> table = this.tableService.get(
Expand All @@ -680,6 +684,7 @@ public TableDto getTable(
.disableOnReadMetadataIntercetor(false)
.includeMetadataFromConnector(includeInfoDetails)
.includeMetadataLocationOnly(includeMetadataLocationOnly)
.includeParentChildInfo(includeParentChildInfo)
.useCache(true)
.build()
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,5 @@ public class GetTableServiceParameters {
private final boolean useCache;
private final boolean includeMetadataFromConnector;
private final boolean includeMetadataLocationOnly;
private final boolean includeParentChildInfo;
}
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ public Optional<TableDto> get(final QualifiedName name, final GetTableServicePar
&& definitionMetadata.get().has(ParentChildRelMetadataConstants.PARENT_CHILD_RELINFO)) {
definitionMetadata.get().remove(ParentChildRelMetadataConstants.PARENT_CHILD_RELINFO);
}
if (config.isParentChildGetEnabled()) {
if (config.isParentChildGetEnabled() || getTableServiceParameters.isIncludeParentChildInfo()) {
final ObjectNode parentChildRelObjectNode = createParentChildObjectNode(name);
if (!parentChildRelObjectNode.isEmpty()) {
if (!definitionMetadata.isPresent()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ class TableServiceImplSpec extends Specification {
noExceptionThrown()
}

def "Mock Parent Child Relationship Drop"() {
def "Mock Parent Child Relationship Drop with isParentChildGetEnabled enabled and getParameter disabled"() {
given:
def name = QualifiedName.ofTable("clone", "clone", "child")

Expand Down Expand Up @@ -577,7 +577,7 @@ class TableServiceImplSpec extends Specification {
noExceptionThrown()
}

def "Mock Parent Child Relationship Get"() {
def "Mock Parent Child Relationship Get with GetTableServiceParameters disabled"() {
when:
service.get(name,GetTableServiceParameters.builder().
disableOnReadMetadataIntercetor(false)
Expand Down Expand Up @@ -609,6 +609,39 @@ class TableServiceImplSpec extends Specification {
1 * parentChildRelSvc.isParentTable(name)
}

def "Mock Parent Child Relationship Get with GetTableServiceParameters enabled"() {
when:
service.get(name,GetTableServiceParameters.builder().
disableOnReadMetadataIntercetor(false)
.includeDataMetadata(true)
.includeDefinitionMetadata(true)
.includeParentChildInfo(true)
.build())
then:
1 * connectorManager.getCatalogConfig(_) >> catalogConfig
1 * usermetadataService.getDefinitionMetadataWithInterceptor(_,_) >> Optional.empty()
1 * usermetadataService.getDataMetadata(_) >> Optional.empty()
0 * usermetadataService.getDefinitionMetadata(_) >> Optional.empty()
1 * config.isParentChildGetEnabled() >> false
1 * parentChildRelSvc.getParents(name)
1 * parentChildRelSvc.isParentTable(name)

when:
service.get(name,GetTableServiceParameters.builder().
disableOnReadMetadataIntercetor(false)
.includeDataMetadata(true)
.includeDefinitionMetadata(true)
.build())
then:
1 * connectorManager.getCatalogConfig(_) >> catalogConfig
1 * usermetadataService.getDefinitionMetadataWithInterceptor(_,_) >> Optional.empty()
1 * usermetadataService.getDataMetadata(_) >> Optional.empty()
0 * usermetadataService.getDefinitionMetadata(_) >> Optional.empty()
1 * config.isParentChildGetEnabled() >> true
1 * parentChildRelSvc.getParents(name)
1 * parentChildRelSvc.isParentTable(name)
}

def "Will not throw on Successful Table Update with Failed Get"() {
given:
def updatedTableDto = new TableDto(name: name, serde: new StorageDto(uri: 's3:/a/b/c'))
Expand Down
Loading