Change checkpoint's table DDL, set column measured_by_atum_agent to h… #319

ABLL526 · 2025-02-21T17:06:52Z

Change checkpoint's table DDL, set column measured_by_atum_agent to have NOT NULL constraint.

Closes #242

Release Notes:

Changed checkpoints DDL column measured_by_atum_agent from BOOLEAN to BOOLEAN NOT NULL

…ave NOT NULL constraint with DEFAULT FALSE #242 Made changes: 1. Changed the value of measured_by_atum_agent in Checkpoints DDL to NOT NULL. 2. Changed the value of measured_by_atum_agent in Checkpoints DDL to DEFAULT FALSE. 3. Changed the value of measured_by_atum_agent in Write Checkpoints to DEFAULT FALSE. 4. Changed the tests to accommodate this change, it has been tested successfully.

ABLL526 · 2025-02-21T17:10:42Z

database/src/test/scala/za/co/absa/atum/database/runs/WriteCheckpointIntegrationTests.scala

@@ -159,7 +159,7 @@ class WriteCheckpointIntegrationTests extends DBTestSuite {
      .setParam("i_process_start_time", startTime)
      .setParam("i_process_end_time", endTime)
      .setParam("i_measurements", CustomDBType(measurements, "JSONB[]"))
-      .setParam("i_measured_by_atum_agent", false)
+      //.setParam("i_measured_by_atum_agent", false)


I have simply commented this out to test the if default works. And it does.
I can always change it back or remove the comment altogether.

ABLL526 · 2025-02-21T17:10:51Z

database/src/test/scala/za/co/absa/atum/database/runs/WriteCheckpointIntegrationTests.scala

@@ -235,7 +235,7 @@ class WriteCheckpointIntegrationTests extends DBTestSuite {
        .add("checkpoint_name", "I came before")
        .add("process_start_time", now())
        .add("process_end_time", now())
-        .add("measured_by_atum_agent", false)
+        //.add("measured_by_atum_agent", false)


I have simply commented this out to test the if default works. And it does.
I can always change it back or remove the comment altogether.

ABLL526 · 2025-02-21T17:10:59Z

...src/test/scala/za/co/absa/atum/database/runs/WriteCheckpointOverloadedIntegrationTests.scala

@@ -161,7 +161,7 @@ class WriteCheckpointOverloadedIntegrationTests extends DBTestSuite {
      .setParam("i_process_start_time", startTime)
      .setParam("i_process_end_time", endTime)
      .setParam("i_measurements", CustomDBType(measurements, "JSONB[]"))
-      .setParam("i_measured_by_atum_agent", false)
+      //.setParam("i_measured_by_atum_agent", false)


I have simply commented this out to test the if default works. And it does.
I can always change it back or remove the comment altogether.

ABLL526 · 2025-02-21T17:11:05Z

...src/test/scala/za/co/absa/atum/database/runs/WriteCheckpointOverloadedIntegrationTests.scala

@@ -237,7 +237,7 @@ class WriteCheckpointOverloadedIntegrationTests extends DBTestSuite {
        .add("checkpoint_name", "I came before")
        .add("process_start_time", now())
        .add("process_end_time", now())
-        .add("measured_by_atum_agent", false)
+        //.add("measured_by_atum_agent", false)


I have simply commented this out to test the if default works. And it does.
I can always change it back or remove the comment altogether.

…ave NOT NULL constraint with DEFAULT FALSE #242 Made changes: 1. Changed the value of measured_by_atum_agent in Checkpoints DDL to NOT NULL. 2. Changed the value of measured_by_atum_agent in Checkpoints DDL to DEFAULT FALSE. 3. Changed the value of measured_by_atum_agent in Write Checkpoints to DEFAULT FALSE. 4. Changed the tests to accommodate this change, it has been tested successfully. 5. Made changes to the WriteCheckpoint.scala files.

github-actions · 2025-02-21T17:53:15Z

JaCoCo model module code coverage report - scala 2.13.11

Overall Project	58.76%	🍏

There is no coverage information present for the Files changed

github-actions · 2025-02-21T17:53:16Z

JaCoCo agent module code coverage report - scala 2.13.11

Overall Project	78.2%	🍏

There is no coverage information present for the Files changed

github-actions · 2025-02-21T17:53:17Z

JaCoCo reader module code coverage report - scala 2.13.11

Overall Project	90.86%	🍏

There is no coverage information present for the Files changed

github-actions · 2025-02-21T17:53:18Z

JaCoCo server module code coverage report - scala 2.13.11

Overall Project	68.39%	🍏

There is no coverage information present for the Files changed

benedeki

Sorry, you went far behind what was needed and actually desired.

All that is needed is in runs.checkpoints
measured_by_atum_agent BOOLEAN NOT NULL, default is not needed, but that I leave up to your discretion.
So please roll back all the other changes.

Unfortunately the table change cannot be done the way how you did it either. FLyway is a migration tool. It's sequence of DB modification scripts. And on each deploy it records and remembers where in the sequence it has ended. Therefor it's not possible (working) to change the previous item in the sequence. You have to create a new one that alters the state of the object in its most recent version.
The new script should be from the range of V0.4.0.

…ave NOT NULL constraint with DEFAULT FALSE #242 Made changes: 1. Changed the value of measured_by_atum_agent in Checkpoints DDL to NOT NULL. 2. Changed the value of measured_by_atum_agent in Checkpoints DDL to DEFAULT FALSE as according to the issue request.

ABLL526 · 2025-02-25T11:50:17Z

@benedeki I have kept the DEFAULT FALSE since it is a requirement in the issue, please let me know if you have an issue with this. I can always change it as needed.

lsulak · 2025-02-26T09:23:34Z

database/src/main/postgres/runs/V0.1.0.32__checkpoints.ddl

@@ -20,7 +20,7 @@ CREATE TABLE runs.checkpoints
    checkpoint_name         TEXT NOT NULL,
    process_start_time      TIMESTAMP WITH TIME ZONE NOT NULL,
    process_end_time        TIMESTAMP WITH TIME ZONE,
-    measured_by_atum_agent  BOOLEAN,
+    measured_by_atum_agent  BOOLEAN NOT NULL DEFAULT FALSE,


I'm not sure if this will work like this with Flyway (please familiarze yourself with it: https://documentation.red-gate.com/flyway/getting-started-with-flyway and https://github.com/sbt/flyway-sbt) - this file V0.1.0.32 was already applied on the DB, and we should always implement increments. I propose to use ALTER TABLE statement in a new sql file.

Also, just out of curiosity, did you or someone check the DB on DEV, UAT, and PROD to see if there are, by any chance records with empty values for this column? If yes we have to deal with it (there might not be any such records)

Thanks @lsulak This is useful to know. I will add a seperate sql file with alter table.
No I have not checked the DBs.

Added SQL file with alter table, added the NOT NULL constraint to measured_by_atum_agent.

benedeki · 2025-02-27T09:06:43Z

database/src/main/postgres/runs/V0.3.0.11__alter_checkpoints.ddl

+ * limitations under the License.
+ */
+
+ALTER TABLE runs.checkpoints ALTER COLUMN measured_by_atum_agent SET NOT NULL;


Sorry for nitpicking and mentioning it earlier. Would be the safest to update the field value to 'false wherever it's NULL

also what do you think about default FALSE? I think I'm slightly more inclined to it but we can leave it up to table's 'users' to define basically. I just think that false is a good candidate here also

I have taken these comments into account. I am not sure if what I have done is correct, specfically adding an update table query.

Made amendments to SQL code mentioned by PR. - Added Update table - Added Default false to table

ABLL526 · 2025-02-27T14:32:54Z

database/src/main/postgres/runs/V0.3.0.11__alter_checkpoints.ddl

+ UPDATE runs.checkpoints
+ SET measured_by_atum_agent = FALSE
+ WHERE measured_by_atum_agent IS NULL;
+


I am very unsure about having an update function in this SQL. Please correct me as needed.

No worries, it's exactly what I was after.
This way it won't fail.

lsulak

I think it's okay now, I'd like the opinion of @benedeki before merging ideally though

ABLL526 added the DB Issues touching the Database part of the project label Feb 21, 2025

ABLL526 self-assigned this Feb 21, 2025

ABLL526 requested review from benedeki, lsulak, Zejnilovic, dk1844 and salamonpavel as code owners February 21, 2025 17:06

ABLL526 linked an issue Feb 21, 2025 that may be closed by this pull request

Change checkpoint's table DDL, set column measured_by_atum_agent to have NOT NULL constraint with DEFAULT FALSE #242

Closed

ABLL526 removed a link to an issue Feb 21, 2025

Change checkpoint's table DDL, set column measured_by_atum_agent to have NOT NULL constraint with DEFAULT FALSE #242

Closed

ABLL526 linked an issue Feb 21, 2025 that may be closed by this pull request

Change checkpoint's table DDL, set column measured_by_atum_agent to have NOT NULL constraint with DEFAULT FALSE #242

Closed

ABLL526 commented Feb 21, 2025

View reviewed changes

benedeki reviewed Feb 25, 2025

View reviewed changes

lsulak reviewed Feb 26, 2025

View reviewed changes

Changes made:

e5a07d7

Added SQL file with alter table, added the NOT NULL constraint to measured_by_atum_agent.

benedeki reviewed Feb 27, 2025

View reviewed changes

Changes made:

72644b5

Made amendments to SQL code mentioned by PR. - Added Update table - Added Default false to table

ABLL526 commented Feb 27, 2025

View reviewed changes

lsulak approved these changes Feb 28, 2025

View reviewed changes

benedeki approved these changes Feb 28, 2025

View reviewed changes

ABLL526 merged commit d9099aa into master Feb 28, 2025
9 checks passed

ABLL526 deleted the #242-Change-checkpoint's-table-DDL branch February 28, 2025 14:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change checkpoint's table DDL, set column measured_by_atum_agent to h… #319

Change checkpoint's table DDL, set column measured_by_atum_agent to h… #319

ABLL526 commented Feb 21, 2025 •

edited

Loading

ABLL526 Feb 21, 2025

ABLL526 Feb 21, 2025

ABLL526 Feb 21, 2025

ABLL526 Feb 21, 2025

github-actions bot commented Feb 21, 2025

github-actions bot commented Feb 21, 2025

github-actions bot commented Feb 21, 2025

github-actions bot commented Feb 21, 2025 •

edited

Loading

benedeki left a comment

ABLL526 commented Feb 25, 2025

lsulak Feb 26, 2025 •

edited

Loading

lsulak Feb 26, 2025

ABLL526 Feb 26, 2025

benedeki Feb 27, 2025

lsulak Feb 27, 2025 •

edited

Loading

ABLL526 Feb 27, 2025

ABLL526 Feb 27, 2025

benedeki Feb 28, 2025

lsulak left a comment

Change checkpoint's table DDL, set column measured_by_atum_agent to h… #319

Change checkpoint's table DDL, set column measured_by_atum_agent to h… #319

Conversation

ABLL526 commented Feb 21, 2025 • 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

github-actions bot commented Feb 21, 2025

JaCoCo model module code coverage report - scala 2.13.11

github-actions bot commented Feb 21, 2025

JaCoCo agent module code coverage report - scala 2.13.11

github-actions bot commented Feb 21, 2025

JaCoCo reader module code coverage report - scala 2.13.11

github-actions bot commented Feb 21, 2025 • edited Loading

JaCoCo server module code coverage report - scala 2.13.11

benedeki left a comment

Choose a reason for hiding this comment

ABLL526 commented Feb 25, 2025

lsulak Feb 26, 2025 • 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

lsulak Feb 27, 2025 • 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

lsulak left a comment

Choose a reason for hiding this comment

ABLL526 commented Feb 21, 2025 •

edited

Loading

github-actions bot commented Feb 21, 2025 •

edited

Loading

lsulak Feb 26, 2025 •

edited

Loading

lsulak Feb 27, 2025 •

edited

Loading