Skip to content

Event Grid Performance Test#19057

Closed
YijunXieMS wants to merge 17 commits intoAzure:mainfrom
YijunXieMS:eg-perf
Closed

Event Grid Performance Test#19057
YijunXieMS wants to merge 17 commits intoAzure:mainfrom
YijunXieMS:eg-perf

Conversation

@YijunXieMS
Copy link
Contributor

No description provided.

@YijunXieMS YijunXieMS added Event Grid Client This issue points to a problem in the data-plane of the library. labels Feb 8, 2021
@YijunXieMS YijunXieMS requested review from g2vinay and srnagar February 8, 2021 19:23
@YijunXieMS YijunXieMS self-assigned this Feb 8, 2021
<packaging>jar</packaging>

<properties>
<maven.compiler.target>11</maven.compiler.target>
Copy link
Member

Choose a reason for hiding this comment

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

this shouldn't be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed to inherit from parent


@Override
public void run() {
eventGridEventPublisherClient.sendEventGridEvents(createEvents());
Copy link
Member

Choose a reason for hiding this comment

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

The create Events method needs to be moved to setupAsync method or the constructor, store the events as a local Array List.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer calling createEvents for every send because it's more like user scenario, which send out different events for every call.


@Override
public void run() {
cloudEventPublisherClient.sendCloudEvents(createEvents());
Copy link
Member

Choose a reason for hiding this comment

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

The create Events method needs to be moved to setupAsync method or the constructor, store the events as a local Array List.

protected final EventGridPublisherClient eventGridEventPublisherClient;
protected final EventGridPublisherAsyncClient eventGridEventPublisherAsyncClient;

public ServiceTest(TOptions options) {
Copy link
Member

Choose a reason for hiding this comment

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

Add some javadocs everywehre, just as best practice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added javadocs (pretty simple though) to pass check style.

SendCloudEventsTest.class,
SendEventGridEventsTest.class
};

Copy link
Member

Choose a reason for hiding this comment

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

This section can be replaced with:
PerfStressProgram.run(new Class<?>[]{
SendCloudEventsTest.class,
SendEventGridEventsTest.class
}, args)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update as suggested.

@JonathanGiles
Copy link
Member

@YijunXieMS @g2vinay Let's get this ready to merge and reviewed - it's a shame to see it not getting put to good use! :-)

List<EventGridEvent> events = new ArrayList<>();
for (int i = 0; i < options.getCount(); i++) {
String dataPayload = String.join("", Collections.nCopies(options.getCount(), "A"));
EventGridEvent event = new EventGridEvent("https://www.eventgrid.com/", "EG.Perf",
Copy link
Member

Choose a reason for hiding this comment

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

should we be using this uri?

@joshfree
Copy link
Member

@srnagar can you help shepherd this PR in?

@srnagar
Copy link
Member

srnagar commented Aug 30, 2021

@joshfree yes, I'll fix the merge conflicts, address comments and merge the PR.

@ghost ghost added the no-recent-activity There has been no recent activity on this issue. label Feb 25, 2022
@ghost
Copy link

ghost commented Feb 25, 2022

Hi @YijunXieMS. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

@ghost ghost closed this Mar 4, 2022
@ghost
Copy link

ghost commented Mar 4, 2022

Hi @YijunXieMS. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing "/reopen" if you'd like to continue working on these changes. Please be sure to use the command to reopen or remove the "no-recent-activity" label; otherwise, this is likely to be closed again with the next cleanup pass.

azure-sdk pushed a commit to azure-sdk/azure-sdk-for-java that referenced this pull request May 16, 2022
[Hub Generated] Publish private branch 'appplatform/release/05-01-preview' (Azure#19057)

* [AutoSync] bff534817 Merged PR 6051224: feat: add acis for lifecycle multi-version config

* revert changes

Co-authored-by: swagger-automation <swagger@microsoft.com>
Co-authored-by: Xuyang Cao <xuycao@microsoft.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Client This issue points to a problem in the data-plane of the library. Event Grid no-recent-activity There has been no recent activity on this issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants