Codegen conversation - #646
Conversation
germanattanasio
left a comment
germanattanasio
left a comment
There was a problem hiding this comment.
createdandupdatedare 99.999% of the timeDate- If we use
Builderwe addOptionsat the end of the name DoubletoStringshould beString.valueOf(Double)- Javadoc adds
:after field name UpdateExample,UpdateIntentandUpdateEntityare just classes to wrap atextproperty. Can we do something better?
| /** The description of the intent. */ | ||
| private String description; | ||
| /** The timestamp for creation of the intent. */ | ||
| private String created; |
There was a problem hiding this comment.
created and updated are Date
There was a problem hiding this comment.
Sadly, the swagger says that they are string. I'll try modifying the swagger and see if the generator will produce the right thing.
There was a problem hiding this comment.
They are String but we should thread them as Date in Java
| /** | ||
| * UpdateExample. | ||
| */ | ||
| public class UpdateExample extends GenericModel { |
There was a problem hiding this comment.
I think we can go with the simple text instead of the builder.
There was a problem hiding this comment.
The problem here is that the updateExample operation already takes a text parameter that identifies the current example, so we can't transfer the text property from UpdateExample to the parameter list, since it would conflict. The generator would make this transformation if there were no conflict, so we should rename one or the other of these in the swagger if that's the effect we want.
| /** Any metadata that is required by the workspace. */ | ||
| private Map<String,Object> metadata; | ||
| /** The timestamp for creation of the workspace. */ | ||
| private String created; |
| /** The timestamp for creation of the workspace. */ | ||
| private String created; | ||
| /** The timestamp for the last update to the workspace. */ | ||
| private String updated; |
| /** The language of the workspace. */ | ||
| private String language; | ||
| /** Any metadata that is required by the workspace. */ | ||
| private Map<String,Object> metadata; |
There was a problem hiding this comment.
Space between String and Object
There was a problem hiding this comment.
I will fix by hand (since this was modified by hand)
| /** The timestamp for creation of the workspace. */ | ||
| private String created; | ||
| /** The timestamp for the last update to the workspace. */ | ||
| private String updated; |
| /** | ||
| * UpdateWorkspace. | ||
| */ | ||
| public class UpdateWorkspace extends GenericModel { |
There was a problem hiding this comment.
I like to use Options if is a builder UpdateWorkspaceOptions
There was a problem hiding this comment.
We could do that but then it should be changed in the swagger. That is how it is handled in NLU. AnalyzeOptions, CategoriesOptions, EmotionOptions, EntitiesOptions etc all are named this way in the NLU swagger doc.
| /** | ||
| * UpdateIntent. | ||
| */ | ||
| public class UpdateIntent extends GenericModel { |
There was a problem hiding this comment.
UpdateIntentOptions
| * | ||
| * List the counterexamples for a workspace. Counterexamples are examples that have been marked as irrelevant input. | ||
| * | ||
| * @param workspaceId: The workspace ID. |
| * @param body: Valid JSON data defining the content of the new workspace. | ||
| * @return the {@link WorkspaceResponse} with the response | ||
| */ | ||
| public ServiceCall<WorkspaceResponse> createWorkspace(CreateWorkspace body) { |
There was a problem hiding this comment.
CreateWorkspaceOptions
|
We need to:
|
|
Regarding
|
|
@mkistler I can merge this once we update the models to have |
|
@mkistler is going to generate the regenerate the code |
2 participants
Summary
Add new APIs for Conversation V1, generated using SDK generator, and hand-written integration tests.