Add support for new Flink table creation API #277
Conversation
Fails in code that is not touched. I'll add a cleanup commit.
Later PR, this is for having the support for the new API. |
jjaakola-aiven
force-pushed
the
jjaakola-aiven-add-new-flink-table-creation-api-support
branch
from
2cdf74b
to
a99d7ea
Compare
aiven/client/cliarg.py
Outdated
|
|
||
| @wraps(fun) | ||
| def wrapped(self): | ||
| sanitized_param_name = param_name.replace("--", "").replace("-", "_") |
There was a problem hiding this comment.
Why is this replace("-", "_") needed? For example if the param name in the request is schema_sql, is the idea here that schema-sql gets converted into schema_sql? Why wouldn't the caller have schema_sql in the first place. That's the name of the parameter that can be seen in the API docs.
What I think that could be the idea is that before this PR the switch to use is --schema-sql and thus it's kind of logical to support schema-sql in the JSON.
But I don't this should be done, since the name of the parameter in the API is not schema-sql.
And why replace("--", "")?
There was a problem hiding this comment.
The - to _ is replaced as it will be the name of the member in the self.args.
The replace for -- is the same as in the user_config_json but with different flavor as the parameter name is known there. In here the parameter name is not known and sanitazion is done programmatically.
See here:
aiven-client/aiven/client/cliarg.py
Lines 89 to 92 in a99d7ea
What could be done is to move the logic when calling arg on line 50. Or change have specialized function for --table-properties like what is done with --user-config-json.
aiven/client/cli.py
Outdated
| ) | ||
| @arg("-s", "--schema-sql", required=True, help="Source/Sink table schema") | ||
| @arg("-s", "--schema-sql", required=False, help="Source/Sink table schema (deprecated)") | ||
| @arg.json_path_or_string_parameter("--table-properties") |
There was a problem hiding this comment.
Mo hint of the format for the JSON? Should there be - it's pretty complex.
And no client side validation?
There was a problem hiding this comment.
Nothing apart of valid JSON check.
jjaakola-aiven
force-pushed
the
jjaakola-aiven-add-new-flink-table-creation-api-support
branch
6 times, most recently
from
15f8430
to
0f6977b
Compare
jjaakola-aiven
force-pushed
the
jjaakola-aiven-add-new-flink-table-creation-api-support
branch
from
782c809
to
8bceb92
Compare
juha-aiven
deleted the
jjaakola-aiven-add-new-flink-table-creation-api-support
branch
About this change: What it does, why it matters
This PR adds the support of new table creation API for Aiven for Apache Flink service.
Command line has new parameter
--table-propertieswhich takes a JSON string or a path to JSON file.Old API is removed.