phet commented on code in PR #3978:
URL: https://github.com/apache/gobblin/pull/3978#discussion_r1644050989
##########
gobblin-runtime/src/test/java/org/apache/gobblin/runtime/spec_store/MysqlBaseSpecStoreTest.java:
##########
@@ -24,6 +24,8 @@
import java.util.Iterator;
import java.util.List;
+import org.apache.gobblin.runtime.api.FlowSpec;
+import org.intellij.lang.annotations.Flow;
Review Comment:
mistaken import?
##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_store/MysqlSpecStore.java:
##########
@@ -63,7 +63,8 @@ public class MysqlSpecStore extends MysqlBaseSpecStore {
// record may contain data there... though in practice, all should, given
the passage of time (amidst the usual retention expiry).
protected static final String SPECIFIC_INSERT_STATEMENT = "INSERT INTO %s
(spec_uri, flow_group, flow_name, template_uri, "
+ "user_to_proxy, source_identifier, destination_identifier, schedule,
tag, isRunImmediately, owning_group, spec, spec_json) "
- + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON DUPLICATE KEY
UPDATE spec = VALUES(spec), spec_json = VALUES(spec_json)";
+ + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON DUPLICATE KEY
UPDATE template_uri = VALUES(template_uri),user_to_proxy =
VALUES(user_to_proxy),source_identifier = VALUES(source_identifier),schedule =
VALUES(schedule),tag = VALUES(tag),isRunImmediately =
VALUES(isRunImmediately),owning_group = VALUES(owning_group),"
Review Comment:
I wasn't familiar w/ the `ON DUPLICATE KEY` ... `VALUES()` function. when I
looked it up, it appears to be deprecated and row/column aliases are to be used
instead. see: https://dev.mysql.com/doc/refman/8.4/en/insert-on-duplicate.html
looks like `destination_identifier` may have been omitted. keeping these in
the same order as they're listed prior (right after `INSERT INTO %s`) might
make that more noticeable
finally, as in java source, leave a space after `,` in SQL
##########
gobblin-runtime/src/test/java/org/apache/gobblin/runtime/spec_store/MysqlBaseSpecStoreTest.java:
##########
@@ -116,6 +135,24 @@ public void testAddSpec() throws Exception {
Assert.assertFalse(this.specStore.exists(URI.create("dummy")));
}
+ @Test
+ public void testUpdateSpec() throws Exception {
+ this.specStore.addSpec(this.flowSpec);
+ Assert.assertEquals(this.specStore.getSize(), 1);
+ Assert.assertTrue(this.specStore.exists(this.uri1));
+ Assert.assertFalse(this.specStore.exists(URI.create("dummy")));
+
+ //Updating
+
+ this.specStore.addSpec(this.flowSpecUpdated);
+ Assert.assertEquals(this.specStore.getSize(), 1);
+ Assert.assertTrue(this.specStore.exists(this.uri1));
+ FlowSpec updated = (FlowSpec)this.specStore.getSpecImpl(this.uri1);
+
Assert.assertEquals(updated.getConfigAsProperties().getProperty("key"),"valueUpdated");
+
Assert.assertEquals(updated.getConfigAsProperties().getProperty("key3"),"value3UPdated");
+
Assert.assertEquals(updated.getConfigAsProperties().getProperty("config.with.dot"),"value4Updated");
Review Comment:
none of these verify any of the special "indexing" columns you specified in
the `ON DUPLICATE KEY` clause, such as `user_to_proxy`, `source_identifier`,
etc.
being the crux of this PR, that would seem worth adding
##########
gobblin-runtime/src/test/java/org/apache/gobblin/runtime/spec_store/MysqlBaseSpecStoreTest.java:
##########
@@ -116,6 +135,24 @@ public void testAddSpec() throws Exception {
Assert.assertFalse(this.specStore.exists(URI.create("dummy")));
}
+ @Test
+ public void testUpdateSpec() throws Exception {
+ this.specStore.addSpec(this.flowSpec);
+ Assert.assertEquals(this.specStore.getSize(), 1);
+ Assert.assertTrue(this.specStore.exists(this.uri1));
+ Assert.assertFalse(this.specStore.exists(URI.create("dummy")));
Review Comment:
what are you going for here? seems unrelated to updating specs...
##########
gobblin-runtime/src/test/java/org/apache/gobblin/runtime/spec_store/MysqlBaseSpecStoreTest.java:
##########
@@ -24,6 +24,8 @@
import java.util.Iterator;
import java.util.List;
+import org.apache.gobblin.runtime.api.FlowSpec;
Review Comment:
see import ordering here -
https://gobblin.apache.org/docs/developer-guide/CodingStyle/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
