feat(doc handle): documentation handling for bedrock #3751

DenovVasil · 2024-12-05T08:18:15Z

Description

Added document handling for bedrock.

documentation PR
e2e PR
demo link

Related issues

https://github.com/camunda/team-connectors/issues/969

...rs/aws/aws-bedrock/src/main/java/io/camunda/connector/aws/bedrock/mapper/DocumentMapper.java

mathias-vandaele · 2024-12-19T10:36:14Z

...ctors/aws/aws-bedrock/src/main/java/io/camunda/connector/aws/bedrock/model/ConverseData.java

+
+  private MessageMapper createMessageMapper() {
+    var bedrockContentMapper = new BedrockContentMapper(new DocumentMapper());
+    return new MessageMapper(bedrockContentMapper);


So we are not returning the new Message and the history but only the history ?
That requires changes in the documentation, and is it what is expected ?

Yes and Yes ))
I described this in the documentation pr

mathias-vandaele · 2024-12-19T10:49:40Z

...ctors/aws/aws-bedrock/src/main/java/io/camunda/connector/aws/bedrock/model/ConverseData.java

-            .toList();
+    var messageMapper = createMessageMapper();
+    var bedrockContentMapper = messageMapper.getBedrockContentMapper();
+    this.messagesHistory.add(prepareNewBedrockMessage(bedrockContentMapper));


Having prepareNewBedrockMessage be part of MessageMapper would be clearer

make sense, I will do ))

johnBgood

A few comments only great work!

johnBgood · 2024-12-19T15:40:08Z

connectors/aws/aws-bedrock/element-templates/aws-bedrock-outbound-connector.json

probably done but just in case: remember the webmodeler pr :)

johnBgood · 2024-12-19T15:45:51Z

...rs/aws/aws-bedrock/src/main/java/io/camunda/connector/aws/bedrock/mapper/DocumentMapper.java

+
+public class DocumentMapper {
+
+  public static final String UNSUPPORTED_DOC_TYPE_MSG = "Unsupported document type: ";


could be "Unsupported document type: {}" and then in the logger.debug(UNSUPPORTED_DOC_TYPE_MSG, fileType)

johnBgood · 2024-12-19T15:46:15Z

...rs/aws/aws-bedrock/src/main/java/io/camunda/connector/aws/bedrock/mapper/DocumentMapper.java

+      fileType = fileType.isEmpty() ? FileUtil.defineType(contentType) : fileType;
+    } catch (MimeTypeException e) {
+      String errorMsg = UNSUPPORTED_CONTENT_TYPE_MSG + contentType;
+      LOGGER.debug(errorMsg);


I don't have the context, but are we sure it's only a debug level message? not even warn ?

johnBgood · 2024-12-19T15:49:52Z

...ors/aws/aws-bedrock/src/main/java/io/camunda/connector/aws/bedrock/model/BedrockContent.java

+import java.util.Objects;
+
+@JsonInclude(JsonInclude.Include.NON_NULL)
+public class BedrockContent {


this can be a record

johnBgood · 2024-12-19T15:50:03Z

...ors/aws/aws-bedrock/src/main/java/io/camunda/connector/aws/bedrock/model/BedrockMessage.java

+import java.util.List;
+import java.util.Objects;
+
+public class BedrockMessage {


this can be a record

johnBgood · 2024-12-19T15:53:48Z

connectors/aws/aws-bedrock/src/main/java/io/camunda/connector/aws/bedrock/util/FileUtil.java

+
+  public static String defineType(String contentType) throws MimeTypeException {
+    MimeTypes mimeTypes = MimeTypes.getDefaultMimeTypes();
+    String extension = mimeTypes.forName(contentType).getExtension();


can mimeTypes.forName(contentType) throw an exception? if so shouldn't we handle it properly and wrap it?

Szik · 2025-01-08T14:22:20Z

QA testing done.
findings:

when defining a single document, the list-brakets "[]" are still needed or the following error is thown:

{"type":"io.camunda.connector.api.error.ConnectorException","message":"Cannot deserialize value of type `java.util.ArrayList<io.camunda.document.Document>` from Object value (token `JsonToken.START_OBJECT`)","code":"JSON_MISMATCH_ERROR"}

if this is intended, maybe this needs to be mentioned in the documentation

a returned error by the connector does not result in an incident error in operate. is this intended?

sbuettner · 2025-01-09T14:26:44Z

@DenovVasil Can you make use of "Object" as the type and figure out whether its a single object document or multiple ones in the Connector?

DenovVasil · 2025-01-09T15:14:34Z

QA testing done. findings:

when defining a single document, the list-brakets "[]" are still needed or the following error is thown:
{"type":"io.camunda.connector.api.error.ConnectorException","message":"Cannot deserialize value of type `java.util.ArrayList<io.camunda.document.Document>` from Object value (token `JsonToken.START_OBJECT`)","code":"JSON_MISMATCH_ERROR"}
if this is intended, maybe this needs to be mentioned in the documentation

a returned error by the connector does not result in an incident error in operate. is this intended?

We can provide a list of documents on the API bedrock, so for me this is expected behavior.
Example:

Upload 2 documents via postman.
2)Make request with both files.
Get response

DenovVasil · 2025-01-09T15:20:01Z

QA testing done. findings:

when defining a single document, the list-brakets "[]" are still needed or the following error is thown:
{"type":"io.camunda.connector.api.error.ConnectorException","message":"Cannot deserialize value of type `java.util.ArrayList<io.camunda.document.Document>` from Object value (token `JsonToken.START_OBJECT`)","code":"JSON_MISMATCH_ERROR"}
if this is intended, maybe this needs to be mentioned in the documentation

a returned error by the connector does not result in an incident error in operate. is this intended?

The documentation states that the documents are a list.

DenovVasil requested a review from a team as a code owner December 5, 2024 08:18

DenovVasil marked this pull request as draft December 5, 2024 08:18

DenovVasil self-assigned this Dec 5, 2024

DenovVasil changed the title ~~969 add doc handling for bedrock and textract~~ feat(doc handle): for bedrock and textract Dec 5, 2024

DenovVasil force-pushed the 969-add-doc-handling-for-bedrock-and-textract branch from 7cb6e55 to b4c6bf5 Compare December 12, 2024 08:41

github-advanced-security bot found potential problems Dec 12, 2024

View reviewed changes

...rs/aws/aws-bedrock/src/main/java/io/camunda/connector/aws/bedrock/mapper/DocumentMapper.java Fixed Show fixed Hide fixed

DenovVasil force-pushed the 969-add-doc-handling-for-bedrock-and-textract branch from b4c6bf5 to 27f5854 Compare December 18, 2024 22:23

DenovVasil requested review from johnBgood and mathias-vandaele December 19, 2024 07:36

DenovVasil marked this pull request as ready for review December 19, 2024 07:36

DenovVasil force-pushed the 969-add-doc-handling-for-bedrock-and-textract branch from 27f5854 to 203f6de Compare December 19, 2024 07:42

DenovVasil added this to the 8.7.0-alpha3 milestone Dec 19, 2024

DenovVasil changed the title ~~feat(doc handle): for bedrock and textract~~ feat(doc handle): documentation handling for bedrock Dec 19, 2024

github-advanced-security bot found potential problems Dec 19, 2024

View reviewed changes

...rs/aws/aws-bedrock/src/main/java/io/camunda/connector/aws/bedrock/mapper/DocumentMapper.java Dismissed Show dismissed Hide dismissed

mathias-vandaele reviewed Dec 19, 2024

View reviewed changes

feat(doc handle): doc handling for bedrock

0c1e39a

DenovVasil force-pushed the 969-add-doc-handling-for-bedrock-and-textract branch from 203f6de to 0c1e39a Compare December 19, 2024 13:34

mathias-vandaele approved these changes Dec 19, 2024

View reviewed changes

DenovVasil added this pull request to the merge queue Dec 19, 2024

johnBgood approved these changes Dec 19, 2024

View reviewed changes

Merged via the queue into main with commit 11b0f42 Dec 19, 2024
13 checks passed

DenovVasil deleted the 969-add-doc-handling-for-bedrock-and-textract branch December 19, 2024 16:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(doc handle): documentation handling for bedrock #3751

feat(doc handle): documentation handling for bedrock #3751

DenovVasil commented Dec 5, 2024 •

edited

Loading

mathias-vandaele Dec 19, 2024

DenovVasil Dec 19, 2024

mathias-vandaele Dec 19, 2024

DenovVasil Dec 19, 2024

DenovVasil Dec 19, 2024

johnBgood left a comment

johnBgood Dec 19, 2024

johnBgood Dec 19, 2024

johnBgood Dec 19, 2024

johnBgood Dec 19, 2024

johnBgood Dec 19, 2024

johnBgood Dec 19, 2024

Szik commented Jan 8, 2025

sbuettner commented Jan 9, 2025

DenovVasil commented Jan 9, 2025

DenovVasil commented Jan 9, 2025


		public class DocumentMapper {

		public static final String UNSUPPORTED_DOC_TYPE_MSG = "Unsupported document type: ";

feat(doc handle): documentation handling for bedrock #3751

feat(doc handle): documentation handling for bedrock #3751

Conversation

DenovVasil commented Dec 5, 2024 • edited Loading

Description

Related issues

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnBgood left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Szik commented Jan 8, 2025

sbuettner commented Jan 9, 2025

DenovVasil commented Jan 9, 2025

DenovVasil commented Jan 9, 2025

DenovVasil commented Dec 5, 2024 •

edited

Loading