Issue #157: Create new SQL Source #224
Conversation
Creation of new type of extension: SourceComputationExtension Creation of new SQL extension for SQL Sources
TNohaic
requested review from
MedMaalej,
NassimBtk,
MarcDouguedroit,
bertysentry,
NathalieLe,
alexdgilbert,
CherfaElyes and
SafaeAJ
| @Builder | ||
| @AllArgsConstructor | ||
| @NoArgsConstructor | ||
| public class SqlColumn { |
There was a problem hiding this comment.
SqlColumn needs to implement Serializable.
Add
private static final long serialVersionUID = 1L;
| * @return A new instance of {@link SqlTable} with the same source, alias and columns. | ||
| */ | ||
| public SqlTable copy() { | ||
| return SqlTable.builder().source(source).alias(alias).columns(columns).build(); |
There was a problem hiding this comment.
return SqlTable.builder().source(source).alias(alias).columns(new ArrayList<>(columns)).build();
public SqlTable copy() {
return SqlTable.builder().source(source).alias(alias).columns(columns.stream().map(SqlColumn::copy).collect(Collectors.toCollection(ArrayList::new))).build();
}| stringBuilder.append(name); | ||
| stringBuilder.append(", type="); | ||
| stringBuilder.append(type); | ||
| stringBuilder.append(", column number="); |
There was a problem hiding this comment.
stringBuilder.append(", number=");
There was a problem hiding this comment.
That's how I set it at first, but I have found that it was not explicit at all when I was lloking at the logs.
There was a problem hiding this comment.
Not a big deal, keep it as it is 😉.
| @NoArgsConstructor | ||
| public class SqlTable implements Serializable { | ||
|
|
||
| private static final long serialVersionUID = 2338817623743864427L; |
There was a problem hiding this comment.
private static final long serialVersionUID = 1L;
| .forceSerialization(forceSerialization) | ||
| .computes(getComputes() != null ? new ArrayList<>(getComputes()) : null) | ||
| .executeForEachEntryOf(executeForEachEntryOf != null ? executeForEachEntryOf.copy() : null) | ||
| .tables(tables != null ? new ArrayList<>(tables) : null) |
There was a problem hiding this comment.
.tables(tables != null ? tables.stream().map(SqlTable::copy).collect(Collectors.toCollection(ArrayList::new)) : null)| for (final List<String> row : table) { | ||
| final List<String> rowValues = new ArrayList<>(); | ||
| for (final SqlColumn sqlColumn : sqlColumns) { | ||
| final String separator = sqlColumn.getType().contains("VARCHAR") ? "'" : ""; |
There was a problem hiding this comment.
Getting the value at the beginning is better String value = row.get(sqlColumn.getNumber() - 1);
|
|
||
| @Override | ||
| public SourceTable processSource(Source source, String connectorId, TelemetryManager telemetryManager) { | ||
| final String hostname = telemetryManager.getHostConfiguration().getHostname(); |
There was a problem hiding this comment.
final String hostname = telemetryManager.getHostname();
|
|
||
| SourceTable sourceTable = new SourceTable(); | ||
|
|
||
| if (executeSqlQuery != null) { |
There was a problem hiding this comment.
.executeQuery(sqlTables, query); shouldn't return null table please.
pom.xml
Outdated
| @@ -342,6 +343,11 @@ | |||
| <artifactId>ssh</artifactId> | |||
| <version>1.0.01</version> | |||
| </dependency> | |||
| <dependency> | |||
There was a problem hiding this comment.
Remove this dependency please , it doesn't exist.
metricshub-sql-extension/pom.xml
Outdated
| <dependency> | ||
| <groupId>com.h2database</groupId> | ||
| <artifactId>h2</artifactId> | ||
| <version>2.2.224</version> |
There was a problem hiding this comment.
Remove the version it is managed in the main pom.
Correction after PR review
| @@ -72,7 +74,7 @@ public class SqlTable implements Serializable { | |||
| * @return A new instance of {@link SqlTable} with the same source, alias and columns. | |||
| */ | |||
| public SqlTable copy() { | |||
| return SqlTable.builder().source(source).alias(alias).columns(columns).build(); | |||
| return SqlTable.builder().source(source).alias(alias).columns(new ArrayList<>(columns)).build(); | |||
There was a problem hiding this comment.
public SqlTable copy() {
return SqlTable.builder().source(source).alias(alias).columns(columns.stream().map(SqlColumn::copy).collect(Collectors.toCollection(ArrayList::new))).build();
}More corrections after PR review
Add handling of null values
* Reverted untested changes.
TNohaic
dismissed
NassimBtk’s stale review
The merge-base changed after approval.
Creation of new type of extension: SourceComputationExtension Creation of new SQL extension for SQL Sources