wenjin272 commented on code in PR #373:
URL: https://github.com/apache/flink-agents/pull/373#discussion_r2622281939
##########
api/src/main/java/org/apache/flink/agents/api/resource/ResourceDescriptor.java:
##########
@@ -20,36 +20,59 @@
import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonCreator;
import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonProperty;
-import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonTypeInfo;
import java.util.HashMap;
import java.util.Map;
+import java.util.Objects;
/** Helper class to describe a {@link Resource} */
public class ResourceDescriptor {
- private static final String FIELD_CLAZZ = "clazz";
- private static final String FIELD_INITIAL_ARGUMENTS = "initialArguments";
+ private static final String FIELD_CLAZZ = "java_clazz";
+ private static final String FIELD_PYTHON_CLAZZ = "python_clazz";
+ private static final String FIELD_PYTHON_MODULE = "python_module";
+ private static final String FIELD_INITIAL_ARGUMENTS = "arguments";
@JsonProperty(FIELD_CLAZZ)
private final String clazz;
+ @JsonProperty(FIELD_PYTHON_CLAZZ)
+ private final String pythonClazz;
+
+ @JsonProperty(FIELD_PYTHON_MODULE)
+ private final String pythonModule;
+
// TODO: support nested map/list with non primitive value.
@JsonProperty(FIELD_INITIAL_ARGUMENTS)
- @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS)
private final Map<String, Object> initialArguments;
@JsonCreator
public ResourceDescriptor(
@JsonProperty(FIELD_CLAZZ) String clazz,
+ @JsonProperty(FIELD_PYTHON_MODULE) String pythonModule,
+ @JsonProperty(FIELD_PYTHON_CLAZZ) String pythonClazz,
Review Comment:
Should add doc strings like python side
##########
api/src/main/java/org/apache/flink/agents/api/resource/ResourceDescriptor.java:
##########
@@ -20,36 +20,59 @@
import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonCreator;
import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonProperty;
-import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonTypeInfo;
import java.util.HashMap;
import java.util.Map;
+import java.util.Objects;
/** Helper class to describe a {@link Resource} */
public class ResourceDescriptor {
- private static final String FIELD_CLAZZ = "clazz";
- private static final String FIELD_INITIAL_ARGUMENTS = "initialArguments";
+ private static final String FIELD_CLAZZ = "java_clazz";
+ private static final String FIELD_PYTHON_CLAZZ = "python_clazz";
+ private static final String FIELD_PYTHON_MODULE = "python_module";
+ private static final String FIELD_INITIAL_ARGUMENTS = "arguments";
@JsonProperty(FIELD_CLAZZ)
private final String clazz;
+ @JsonProperty(FIELD_PYTHON_CLAZZ)
+ private final String pythonClazz;
+
+ @JsonProperty(FIELD_PYTHON_MODULE)
+ private final String pythonModule;
+
// TODO: support nested map/list with non primitive value.
@JsonProperty(FIELD_INITIAL_ARGUMENTS)
- @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS)
Review Comment:
If remove this annotation, the jackson can't reconstruct object instance for
custom object instance due to the lack of class information. For example,
`initialArguments` contains a key-value pair `"myObject": new CustomClass()`
Currently, the initialArguments are always primitive types, or map/list of
primitive types, so to keep the consistency with serialization behavior in
python side, we can remove this, but we need revise the TODO.
--
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]
