feat(migrations): add --define flag to apply command #7401
Conversation
| for (String pair : definedVars) { | ||
| final String[] parts = pair.split("="); | ||
| if (parts.length != 2) { | ||
| throw new IllegalArgumentException("Variables must be defined using '=' (i.e. var=val)."); |
There was a problem hiding this comment.
Not your code but let's take this opportunity to improve the error message (the migrations tool displays this error directly so we should make it more informative). Let's include the bad argument in the error message.
...ols/src/test/java/io/confluent/ksql/tools/migrations/commands/ApplyMigrationCommandTest.java
Show resolved
Hide resolved
ksqldb-common/src/main/java/io/confluent/ksql/parser/VariableParser.java
Show resolved
Hide resolved
| createMigrationFile(2, NAME, migrationsDir, "INSERT INTO FOO VALUES ('${name}2');"); | ||
| command = PARSER.parse("-a", "-d", "name=tame", "-d", "dame=blame"); | ||
| createMigrationFile(1, NAME, migrationsDir, "INSERT INTO FOO VALUES ('${name}'); INSERT INTO FOO VALUES ('${dame}');"); | ||
| createMigrationFile(2, NAME, migrationsDir, "DEFINE name='flame'; INSERT INTO FOO VALUES ('${name}');"); |
There was a problem hiding this comment.
Let's have a separate unit test for this piece that verifies the order of precedence between DEFINE statements within migrations files and the command options. (The unit test name does not clarify that this is being tested here, so it's unclear to the reader that this piece of the test is important. Unit tests are meant to be small and quick to run so it doesn't hurt to have more of them, as long as each tests a very targeted piece of functionality.)
Description
Allow users to define variables using the
--defineflag when callingapply. For example,This commit builds on top of #7366, so you can just review the
feat(migrations): add --define flag to apply commandcommit.Testing done
Unit test, updated integration test
Reviewer checklist