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

[CDAP-21093] Add ExternalDocumentationLink in abstract sink #533

Open
wants to merge 1 commit into
base: develop
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 @@ -108,6 +108,11 @@ protected String getErrorDetailsProviderClassName() {
return CloudSQLMySQLErrorDetailsProvider.class.getName();
}

@Override
protected String getExternalDocumentationLink() {
return DBUtils.CLOUDSQLMYSQL_SUPPORTED_DOC_URL;
}

@Override
protected LineageRecorder getLineageRecorder(BatchSinkContext context) {
String host;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ protected String getErrorDetailsProviderClassName() {
return CloudSQLPostgreSQLErrorDetailsProvider.class.getName();
}

@Override
protected String getExternalDocumentationLink() {
return DBUtils.CLOUDSQLPOSTGRES_SUPPORTED_DOC_URL;
}

/** CloudSQL PostgreSQL sink config. */
public static class CloudSQLPostgreSQLSinkConfig extends AbstractDBSpecificSinkConfig {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@
import io.cdap.cdap.api.data.format.StructuredRecord;
import io.cdap.cdap.api.data.schema.Schema;
import io.cdap.cdap.api.dataset.lib.KeyValue;
import io.cdap.cdap.api.exception.ErrorCategory;
import io.cdap.cdap.api.exception.ErrorCodeType;
import io.cdap.cdap.api.exception.ErrorType;
import io.cdap.cdap.api.exception.ErrorUtils;
import io.cdap.cdap.api.plugin.PluginConfig;
import io.cdap.cdap.etl.api.Emitter;
import io.cdap.cdap.etl.api.FailureCollector;
Expand Down Expand Up @@ -175,6 +179,16 @@ protected String getErrorDetailsProviderClassName() {
return DBErrorDetailsProvider.class.getName();
}

/**
* Returns the external documentation link.
* Override this method to provide a custom external documentation link.
*
* @return external documentation link
*/
protected String getExternalDocumentationLink() {
return null;
}

@Override
public void prepareRun(BatchSinkContext context) {
String connectionString = dbSinkConfig.getConnectionString();
Expand Down Expand Up @@ -296,8 +310,21 @@ private Schema inferSchema(Class<? extends Driver> driverClass) {
inferredFields.addAll(getSchemaReader().getSchemaFields(rs));
}
} catch (SQLException e) {
throw new InvalidStageException("Error while reading table metadata", e);

// wrap exception to ensure SQLException-child instances not exposed to contexts w/o jdbc driver in classpath
String errorMessageWithDetails = String.format("Error while reading table metadata." +
"Error message: '%s'. Error code: '%s'. SQLState: '%s'", e.getMessage(), e.getErrorCode(), e.getSQLState());
String externalDocumentationLink = getExternalDocumentationLink();
if (!Strings.isNullOrEmpty(externalDocumentationLink)) {
if (!errorMessageWithDetails.endsWith(".")) {
errorMessageWithDetails = errorMessageWithDetails + ".";
}
errorMessageWithDetails = String.format("%s For more details, see %s", errorMessageWithDetails,
externalDocumentationLink);
}
throw ErrorUtils.getProgramFailureException(new ErrorCategory(ErrorCategory.ErrorCategoryEnum.PLUGIN),
e.getMessage(), errorMessageWithDetails, ErrorType.USER, false, ErrorCodeType.SQLSTATE,
e.getSQLState(), externalDocumentationLink, new SQLException(e.getMessage(),
e.getSQLState(), e.getErrorCode()));

Choose a reason for hiding this comment

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

NIT : remove space

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is auto formatted, with extra intendation since e.getSQLState(), e.getErrorCode()) is part of nested function call to new SQLException

}
} catch (IllegalAccessException | InstantiationException | SQLException e) {
throw new InvalidStageException("JDBC Driver unavailable: " + dbSinkConfig.getJdbcPluginName(), e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,12 @@ protected Class<? extends DBWritable> getDBRecordType() {
return DBRecord.class;
}

/**
* Returns the external documentation link.
* Override this method to provide a custom external documentation link.
*
* @return external documentation link
*/
protected String getExternalDocumentationLink() {

Choose a reason for hiding this comment

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

I think we should make this abstract and enforce the child to implement it correctly.

Returning null might cause issue if someone misses to override this.

return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ protected String getErrorDetailsProviderClassName() {
return MysqlErrorDetailsProvider.class.getName();
}

@Override
protected String getExternalDocumentationLink() {
return DBUtils.MYSQL_SUPPORTED_DOC_URL;
}

/**
* MySQL action configuration.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ protected String getErrorDetailsProviderClassName() {
return PostgresErrorDetailsProvider.class.getName();
}

@Override
protected String getExternalDocumentationLink() {
return DBUtils.POSTGRES_SUPPORTED_DOC_URL;
}

/**
* PostgreSQL action configuration.
*/
Expand Down
Loading