Skip to content
This repository was archived by the owner on Mar 31, 2023. It is now read-only.

Conversation

@luyaoluo
Copy link
Contributor

This is the PR for pulsar client implementation on aca based on the previous designs.

@zzxgzgz
Copy link
Contributor

zzxgzgz commented Oct 20, 2021

Thank you for submitting this PR @lly00,

could you please share the test results, if any, with the community? That will help us a lot in the reviewing process.

Thank you.

@luyaoluo
Copy link
Contributor Author

Thank you for submitting this PR @lly00,

could you please share the test results, if any, with the community? That will help us a lot in the reviewing process.

Thank you.

OK, but what kind of test results do we need? Are the test results of gtests or just of pulsar client?

@zzxgzgz
Copy link
Contributor

zzxgzgz commented Oct 21, 2021

Thank you for submitting this PR @lly00,
could you please share the test results, if any, with the community? That will help us a lot in the reviewing process.
Thank you.

OK, but what kind of test results do we need? Are the test results of gtests or just of pulsar client?

The gtest results will be great. Also, I noticed that the listener function can receive GoalState messages before it calls rc = Aca_Comm_Manager::get_instance().update_goal_state( deserialized_GoalState, gsOperationalReply);. Can you please post the results of that function being called? It will be great to see the pulsar consumer is able to receive GoalState and update GoalState successfully.

Thank you.

@luyaoluo
Copy link
Contributor Author

luyaoluo commented Dec 3, 2021

@zzxgzgz We have add some test cases for pulsar consumer, which send a goalstate to create a port. The pulsar consumer is able to receive GoalState and update GoalState successfully. However, it does not support goalstateV2 currently.
Besides, some test cases in aca_tests can not pass.
image

@zzxgzgz
Copy link
Contributor

zzxgzgz commented Dec 6, 2021

@zzxgzgz We have add some test cases for pulsar consumer, which send a goalstate to create a port. The pulsar consumer is able to receive GoalState and update GoalState successfully. However, it does not support goalstateV2 currently. Besides, some test cases in aca_tests can not pass. image

Thank you for your update.

For this PR, adding support to GoalState(V1) should be good enough.

Can you please provide some kind of test result of the GoalState, just like the screenshot you provided for the GoalStateV2 tests? So that we can confirm that the GoalState was successfully sent to ACA via this pulsar channel.

Thank you.

@luyaoluo
Copy link
Contributor Author

luyaoluo commented Dec 9, 2021

There are the results for GoalStateV2 tests, whicih are in gtests.
producer:
9acdcf339b9373708422f34ad71a206
consumer:
589a4d206b4c36075bcd635497348d1

luyaoluo and others added 2 commits December 9, 2021 10:04
fix pulsar producer orderingKey bugs
@zzxgzgz
Copy link
Contributor

zzxgzgz commented Dec 9, 2021

There are the results for GoalStateV2 tests, whicih are in gtests. producer: 9acdcf339b9373708422f34ad71a206 consumer: 589a4d206b4c36075bcd635497348d1

Thank you for your update. Looks like that the Pulsar is now able to receive GoalStateV2 messages.

@xieus xieus requested review from lfu-ps and zzxgzgz December 10, 2021 20:35
@xieus xieus added enhancement New feature or request Feature labels Dec 10, 2021
Copy link
Contributor

@zzxgzgz zzxgzgz left a comment

Choose a reason for hiding this comment

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

LGTM

@xieus xieus merged commit 74a78cf into futurewei-cloud:master Dec 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request Feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants