-
Notifications
You must be signed in to change notification settings - Fork 1
adding new property TableName in the ui and its functionality #5
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
base: develop
Are you sure you want to change the base?
Conversation
@@ -105,6 +108,25 @@ public List<SnowflakeFieldDescriptor> describeQuery(String query) throws IOExcep | |||
return fieldDescriptors; | |||
} | |||
|
|||
public List<SnowflakeFieldDescriptor> describeTable(String schemaName, String tableName) throws SQLException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add Javadoc for public methods
@@ -193,4 +215,8 @@ private static String writeTextToTmpFile(String text) { | |||
throw new RuntimeException("Cannot write key to temporary file", e); | |||
} | |||
} | |||
|
|||
public String getSchema() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add javadoc here as well
String copy = String.format(COMAND_COPY_INTO, QueryUtil.removeSemicolon(config.getImportQuery())); | ||
String importQuery = config.getImportQuery(); | ||
if (Strings.isNullOrEmpty(importQuery)) { | ||
importQuery = "SELECT * FROM " + config.getTableName(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to use String.format method
|
||
SnowflakeBatchSourceConfig mockConfig = Mockito.mock(SnowflakeBatchSourceConfig.class); | ||
Mockito.when(mockConfig.getSchema()).thenReturn("{}"); | ||
// SchemaHelper.getSchema(null, "{}", collector, null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think its better to not remove the comments it will help other developer to easily see what changes i had made to the particular line of code.
@@ -36,18 +36,24 @@ | |||
import java.io.FileWriter; | |||
import java.io.IOException; | |||
import java.lang.reflect.Field; | |||
//import java.sql.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove comments
/** | ||
* A class which will help in connection | ||
*/ | ||
|
||
public void runSQL(String query) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove javadoc
@@ -136,12 +136,13 @@ public abstract class LoadUnloadConfig extends BaseSnowflakeConfig { | |||
|
|||
|
|||
public LoadUnloadConfig(String accountName, String database, | |||
String schemaName, String username, String password, | |||
String schemaName, String tableName, String username, String password, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tableName is not required here!
@@ -63,6 +64,13 @@ public class BaseSnowflakeConfig extends PluginConfig { | |||
@Macro | |||
private String schemaName; | |||
|
|||
@Name(PROPERTY_TABLE_NAME) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Table Name should be moved under Basic section using widgets.json
@@ -71,11 +71,13 @@ public class LoadActionConfig extends LoadUnloadConfig { | |||
@Nullable | |||
private String pattern; | |||
|
|||
public LoadActionConfig(String accountName, String database, String schemaName, String username, String password, | |||
public LoadActionConfig(String accountName, String database, String schemaName, String tableName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tableName not required here as well
@@ -97,18 +97,17 @@ private static Schema getParsedSchema(String schema) { | |||
} | |||
} | |||
|
|||
public static Schema getSchema(SnowflakeAccessor snowflakeAccessor, String schemaName, | |||
public static Schema getSchema(SnowflakeAccessor snowflakeAccessor, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getSchema method used within the SchemaHelper class only should be made private
@@ -105,7 +105,7 @@ private void validateInputSchema(Schema schema, FailureCollector failureCollecto | |||
|
|||
SnowflakeAccessor snowflakeAccessor = new SnowflakeAccessor(this); | |||
// Schema expectedSchema = SchemaHelper.getSchema(snowflakeAccessor, String.format(GET_FIELDS_QUERY, tableName)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove comments
Schema actual = SchemaHelper.getSchema(mockConfig, collector); | ||
// SnowflakeBatchSourceConfig mockConfig = Mockito.mock(SnowflakeBatchSourceConfig.class); | ||
// Mockito.when(mockConfig.canConnect()).thenReturn(false); | ||
// Mockito.when(mockConfig.getSchema()).thenReturn(expected.toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove comments
adding new property TableName in the ui and its functionality i.e now user can get reterive the data from giving tablename without providing the import query.