CloudSql PostgreSql e2e #12

Vipinofficial11 · 2023-06-01T14:54:27Z

Vipinofficial11 · 2023-06-01T18:35:13Z

To do : Internal Review Pending.

Vipinofficial11 · 2023-06-04T15:56:41Z

cloudsql-postgresql-plugin/pom.xml

@@ -102,6 +102,12 @@
      <version>42.3.1</version>
      <scope>test</scope>
    </dependency>
+      <dependency>
+          <groupId>com.google.code.gson</groupId>


Why do we need this ?

This is basically used to convert java objects into json representation and vice versa so that's why we are using this.

Its already present in google-http-client dependency which is added in cdap-e2e-test repo. Use import com.google.gson.Gson and try if you are able to resolve it without adding a extra dependency for Gson.

Vipinofficial11 · 2023-06-04T15:57:52Z

cloudsql-postgresql-plugin/src/e2e-test/resources/pluginParameters.properties

+zeroValue=0
+splitByColumn=ID
+importQuery =  where $CONDITIONS
+connectionName=CONNECTION_NAME


make it CLOUDSQL_POSTGRESQL_CONNECTION_NAME

Vipinofficial11 · 2023-06-04T16:00:05Z

...ugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/stepsdesign/CloudSqlPostgreSql.java

+import java.text.ParseException;
+
+/**
+ *  CLOUDSQLPOSTGRESQL Plugin related step design.


CloudSQL PostgreSQL Step design sounds better.

Vipinofficial11 · 2023-06-04T16:00:35Z

...ugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/stepsdesign/CloudSqlPostgreSql.java

+ */
+public class CloudSqlPostgreSql implements CdfHelper {
+
+  @Then("Click on preview data for CloudSQLPostgreSQL sink")


Why do we need to write a step here, we can use it from framework.

Vipinofficial11 · 2023-06-04T16:03:45Z

...gin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/runners/sourcerunner/TestRunner.java

+@CucumberOptions(
+  features = {"src/e2e-test/features"},
+  glue = {"io.cdap.plugin.cloudsqlpostgresql.stepsdesign", "stepsdesign", "io.cdap.plugin.common.stepsdesign"},
+  tags = {"@Cloudsqlpostgresql_Source and not @PLUGIN-1526"},


We don't have to write the name of the tag same as plugins name. It looks complex. Can we Just use Regression as a tag.

Change done.

Vipinofficial11 · 2023-06-04T16:04:15Z

...lugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/runners/sinkrunner/TestRunner.java

+@CucumberOptions(
+  features = {"src/e2e-test/features"},
+  glue = {"io.cdap.plugin.cloudsqlpostgresql.stepsdesign", "stepsdesign", "io.cdap.plugin.common.stepsdesign"},
+  tags = {"@Cloudsqlpostgresql_Sink and not @PLUGIN-1629 and not @PLUGIN-1526"},


Same comment here lets make it Regression

Change done.

Vipinofficial11 · 2023-06-04T16:05:08Z

...sql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/CloudSqlPostgreSqlClient.java

+    String password = System.getenv("CLOUDSQL_POSTGRESQL_PASSWORD");
+
+    String jdbcUrl = String.format(
+      "jdbc:postgresql://google/%s?cloudSqlInstance=%s&socketFactory=com.google.cloud.sql.postgres.SocketFactory&user=%s&password=%s",


Don't hardcode this URL here, fetch it from Plugin Parameter.properties

Vipinofficial11 · 2023-06-04T16:05:22Z

...sql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/CloudSqlPostgreSqlClient.java

+      "jdbc:postgresql://google/%s?cloudSqlInstance=%s&socketFactory=com.google.cloud.sql.postgres.SocketFactory&user=%s&password=%s",
+       database,instanceConnectionName, username, password);
+    Connection conn = DriverManager.getConnection(jdbcUrl);
+    System.out.println("Connected to the database successfully");


Remove Sout

Vipinofficial11 · 2023-06-04T16:05:37Z

...sql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/CloudSqlPostgreSqlClient.java

+       database,instanceConnectionName, username, password);
+    Connection conn = DriverManager.getConnection(jdbcUrl);
+    System.out.println("Connected to the database successfully");
+    return conn;


conn -> connection

Vipinofficial11 · 2023-06-04T16:06:40Z

...sql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/CloudSqlPostgreSqlClient.java

+  }
+
+  /**
+   * Extracts entire data from source and target tables.


Extracts Result set of source and target table.

Change done.

Vipinofficial11 · 2023-06-04T16:07:04Z

...sql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/CloudSqlPostgreSqlClient.java

+    throws SQLException, ClassNotFoundException {
+    String getSourceQuery = "SELECT * FROM " + schema + "." + sourceTable;
+    String getTargetQuery = "SELECT * FROM " + schema + "." + targetTable;
+    try (Connection connect = getCloudSqlConnection()) {


connect -> connection

Vipinofficial11 · 2023-06-04T16:09:26Z

...sql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/CloudSqlPostgreSqlClient.java

+    while (rsSource.next() && rsTarget.next()) {
+      int currentColumnCount = 1;
+      while (currentColumnCount <= columnCountSource) {
+        String columnTypeName = mdSource.getColumnTypeName(currentColumnCount);


Is this logic of comparing result set copied from PostgreSQL ? How do we even know we just have to cover timestamp in a separate case and rest we can compare as a string.

Vipinofficial11 · 2023-06-04T16:11:06Z

...dsql-postgresql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/BQValidation.java

+            throws SQLException, ClassNotFoundException, IOException, InterruptedException {
+        getBigQueryTableData(targetTable, bigQueryRows);
+        for (Object rows : bigQueryRows) {
+            JsonObject json = new Gson().fromJson(String.valueOf(rows), JsonObject.class);


You have already used Lists as global variables. Create a global Gson object and re use it in all the methods.

change done

Vipinofficial11 · 2023-06-04T16:11:53Z

...dsql-postgresql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/BQValidation.java

+    /**
+     * Retrieves the data from a specified BigQuery table and populates it into the provided list of objects.
+     *
+     * @param table        The name of the BigQuery table to fetch data from.


Remove the extra spacing.

I don't see any commit. push the commit and add the link here

Vipinofficial11 · 2023-06-04T16:15:16Z

...dsql-postgresql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/BQValidation.java

+                        BigDecimal targetDecimal = bigQueryData.get(jsonObjectIdx).get(columnName).getAsBigDecimal();
+                        int desiredScale = 2; // Set the desired scale (number of decimal places)
+                        BigDecimal adjustedSourceValue = sourceDecimal.setScale(desiredScale, RoundingMode.HALF_UP);
+                        BigDecimal adjustedTargetValue = targetDecimal.setScale(desiredScale, RoundingMode.HALF_UP);


I see we are setting scale here, was there any discrepancy in the original data which was needed to be handled.

We need to match precision scale of both side to avoid errors cause due to auto round off when precision mismatch

example -> 2.126 (3 decimal precision) will be converted to 2.13 (2 decimal precision) (round off)
👍

Vipinofficial11 · 2023-06-04T16:15:47Z

...dsql-postgresql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/BQValidation.java

+                        break;
+
+                    case Types.TIMESTAMP:
+                        break;


As discussed we need to add the logic here we are not going to simply add break here.

Vipinofficial11 · 2023-06-04T16:16:42Z

...dsql-postgresql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/BQValidation.java

+        }
+    }
+
+    private static boolean getTimeValidation(ResultSet rsSource, String bqDateString, String columnName, String


Is this method getting used somewhere now ?

Yes we are using this !

you are using this code Timestamp sourceTS = rsSource.getTimestamp(columnName); SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'"); Date parsedDate = dateFormat.parse(bigQueryData.get(jsonObjectIdx).get(columnName).getAsString()); Timestamp targetTs = new Timestamp(parsedDate.getTime()); Assert.assertEquals(sourceTS, targetTs);

To comapre time stamp data. Then what is this method used for ?

We are using getTimeValidation for Time with and without zone data not for Timestamp

…ql-postgreSql # Conflicts: # cloudsql-postgresql-plugin/src/e2e-test/feature/cloudsql-postgresql/sink/DesignTime.feature # cloudsql-postgresql-plugin/src/e2e-test/feature/cloudsql-postgresql/sink/DesignTimeWithMacros.feature # cloudsql-postgresql-plugin/src/e2e-test/feature/cloudsql-postgresql/sink/DesignTimeWithValidation.feature # cloudsql-postgresql-plugin/src/e2e-test/feature/cloudsql-postgresql/sink/RunTime.feature # cloudsql-postgresql-plugin/src/e2e-test/feature/cloudsql-postgresql/sink/RunTimeMacro.feature # cloudsql-postgresql-plugin/src/e2e-test/feature/cloudsql-postgresql/source/DesignTime.feature # cloudsql-postgresql-plugin/src/e2e-test/feature/cloudsql-postgresql/source/DesignTimeWithMacro.feature # cloudsql-postgresql-plugin/src/e2e-test/feature/cloudsql-postgresql/source/DesignTimeWithValidation.feature # cloudsql-postgresql-plugin/src/e2e-test/feature/cloudsql-postgresql/source/RunTime.feature # cloudsql-postgresql-plugin/src/e2e-test/feature/cloudsql-postgresql/source/RunTimeMacro.feature # cloudsql-postgresql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/BQValidation.java # cloudsql-postgresql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/CloudSqlPostgreSqlClient.java # cloudsql-postgresql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/runners/sinkrunner/TestRunner.java # cloudsql-postgresql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/runners/sourcerunner/TestRunner.java # cloudsql-postgresql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/stepsdesign/CloudSqlPostgreSql.java # cloudsql-postgresql-plugin/src/e2e-test/java/io/cdap/plugin/common/stepsdesign/TestSetUpHooks.java # cloudsql-postgresql-plugin/src/e2e-test/resources/pluginParameters.properties # cloudsql-postgresql-plugin/src/e2e-test/resources/testdata/BigQuery/BigQueryInsertDataQuery.txt

Vipinofficial11 · 2023-06-05T17:28:59Z

cloudsql-postgresql-plugin/src/e2e-test/java/io/cdap/plugin/CloudSqlPostgreSqlClient.java

+        String instanceConnectionName = System.getenv("CLOUDSQL_POSTGRESQL_CONNECTION_NAME");
+        String username = System.getenv("CLOUDSQL_POSTGRESQL_USERNAME");
+        String password = System.getenv("CLOUDSQL_POSTGRESQL_PASSWORD");
+        String jdbcUrl = String.format(PluginPropertyUtils.pluginProp("URL"), database, instanceConnectionName, username, password);


Did you tested the code after adding this URL from plugin properties if its working fine.

yes this is working fine.

Vipinofficial11 · 2023-06-05T17:31:43Z

...dsql-postgresql-plugin/src/e2e-test/java/io/cdap/plugin/cloudsqlpostgresql/BQValidation.java

@@ -40,51 +42,52 @@
 public class BQValidation {
    static List<JsonObject> BigQueryResponse = new ArrayList<>();
    static List<Object> bigQueryRows = new ArrayList<>();
+    static JsonObject json;



Didn't asked to create a JsonObject global . I asked to create this here.
Gson gson = new Gson()
JsonObject json = gson.fromJson(----------);

change done

Vipinofficial11 · 2023-06-09T05:51:09Z

@bharatgulati Changes look good all the comments have been addressed. Raise an external PR after dry running the data validation logic once.

bharatgulati and others added 3 commits May 16, 2023 12:40

Design Time Scenarios (Source and Sink)

92795f1

e2e_CloudsqlPostgresql scenarios

109283b

e2e_Cloudsqlpostgresql run-time changes

f4fa249

Vipinofficial11 changed the title ~~Cloud sql postgre sql~~ CLOUDSQL POSTGRESQL Jun 1, 2023

Vipinofficial11 changed the title ~~CLOUDSQL POSTGRESQL~~ CloudSql PostgreSql e2e Jun 1, 2023

e2e_CloudSQLPostGreSQL Test scenarios

2ae73ac

Vipinofficial11 force-pushed the cloudSql-postgreSql branch from 97d8e50 to 2ae73ac Compare June 1, 2023 18:26

Vipinofficial11 marked this pull request as ready for review June 1, 2023 18:27

Vipinofficial11 commented Jun 4, 2023

View reviewed changes

AnkitCLI force-pushed the cloudSql-postgreSql branch from 351fee6 to 8cdbadd Compare June 5, 2023 10:46

Vipinofficial11 commented Jun 5, 2023

View reviewed changes

initial commits

194d85d

AnkitCLI force-pushed the cloudSql-postgreSql branch from 285d30e to 194d85d Compare June 6, 2023 05:46

CloudSql PostgreSql e2e #12

Are you sure you want to change the base?

CloudSql PostgreSql e2e #12

Conversation

Vipinofficial11 commented Jun 1, 2023 • edited Loading

Vipinofficial11 commented Jun 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vipinofficial11 Jun 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vipinofficial11 commented Jun 9, 2023

Vipinofficial11 commented Jun 1, 2023 •

edited

Loading

Vipinofficial11 Jun 4, 2023 •

edited

Loading