Skip to content

feat(device): support batch messages for HTTPS#889

Merged
azabbasi merged 18 commits intomasterfrom
feature/iot/azabbasi/httpbatch
Aug 12, 2020
Merged

feat(device): support batch messages for HTTPS#889
azabbasi merged 18 commits intomasterfrom
feature/iot/azabbasi/httpbatch

Conversation

@azabbasi
Copy link
Copy Markdown
Contributor

No description provided.

* Internal constructor to initialize a bulk message instance.
* @param messages List of nested messages
*/
Message(Set<Message> messages)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constructor is internal to the iot.device package.

@timtay-microsoft
Copy link
Copy Markdown
Member

So how does IoTHub accept these batch messages if one is malformed? Is it an all-or-nothing type API?

@timtay-microsoft
Copy link
Copy Markdown
Member

If we plan on adding bulk message support to AMQP later, shouldn't we not allow users to call this API over AMQP? Since HTTP is the only protocol that actually currently supports bulk messages, I'd expect to see

if (this.protocol != HTTPS)
{
    throw new UnsupportedOperationException("Bulk messages are only supported over HTTPS");
}

@azabbasi
Copy link
Copy Markdown
Contributor Author

If we plan on adding bulk message support to AMQP later, shouldn't we not allow users to call this API over AMQP? Since HTTP is the only protocol that actually currently supports bulk messages, I'd expect to see

if (this.protocol != HTTPS)
{
    throw new UnsupportedOperationException("Bulk messages are only supported over HTTPS");
}

We sure can, We use the same API in our C# SDK and I tried to stay consistent with that. we can chat offline about this.

@azabbasi
Copy link
Copy Markdown
Contributor Author

So how does IoTHub accept these batch messages if one is malformed? Is it an all-or-nothing type API?

This is an all or nothing API for HTTPS, I will add an e2e test to catch that and have coverage for those cases.

private boolean isBulk;

/**
* List of nested messages.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do nested messages mean here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have moved it to batched messages. That makes the batch message a wrapper around the same object type of Message.

Comment thread device/iot-device-samples/send-batch-events/README.md Outdated
Comment thread device/iot-device-samples/send-batch-events/README.md Outdated
Comment thread device/iot-device-samples/send-batch-events/README.md Outdated
Comment thread device/iot-device-samples/send-event/README.md Outdated
Copy link
Copy Markdown
Member

@timtay-microsoft timtay-microsoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the one comment left, but otherwise looks good!

@azabbasi
Copy link
Copy Markdown
Contributor Author

/azp run Java Prod Basic, Java Prod, SDL, horton-java-gate

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 4 pipeline(s).

@timtay-microsoft
Copy link
Copy Markdown
Member

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 4 pipeline(s).

@azabbasi azabbasi merged commit 0b395e8 into master Aug 12, 2020
@timtay-microsoft timtay-microsoft deleted the feature/iot/azabbasi/httpbatch branch August 24, 2020 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants