Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ConsumerNotifier and ACL schema for PINs / P4Runtime #2

Draft
wants to merge 5 commits into
base: pr2-batch-ops
Choose a base branch
from
Draft

Add ConsumerNotifier and ACL schema for PINs / P4Runtime #2

wants to merge 5 commits into from

Conversation

bocon13
Copy link

@bocon13 bocon13 commented Oct 11, 2021

  • Add Bazel build support so that swss-common can be used
      in Bazel projects; this doesn’t change/impact the
      make-based build system
  • Add ConsumerNotifier for response path
  • Introduce interfaces for DBConnector, NotificationConsumer,
      ConsumerNotifier, NotificationProducer, ProducerStateTable
      to make it easier to test these classes
  • Add AppDB ACL schema values between the P4RT app and P4Orch
  • Add lots of new tests for notification classes

Submission containing materials of a third party:
    Copyright Google LLC; Licensed under Apache 2.0 

Co-authored-by: Akarsh Gupta [email protected]
Co-authored-by: Ashish Singh [email protected]
Co-authored-by: Jay Hu [email protected]
Co-authored-by: Manali Kumar [email protected]
Co-authored-by: Richard Yu [email protected]
Co-authored-by: Robert J. Halstead [email protected]
Co-authored-by: Runming Wu [email protected]
Co-authored-by: Srikishen Pondicherry Shanmugam [email protected]
Co-authored-by: Vivek Ramamoorthy [email protected]
Co-authored-by: Yilan Ji [email protected]

Signed-off-by: Don Newton [email protected]

@bocon13 bocon13 mentioned this pull request Oct 11, 2021
Open
@bocon13
Copy link
Author

bocon13 commented Oct 11, 2021

cc: @mint570

@bocon13 bocon13 marked this pull request as draft October 11, 2021 23:25
bocon13
bocon13 commented Oct 22, 2021
View reviewed changes
tests/test_redis_ut.py Outdated Show resolved Hide resolved
bocon13
bocon13 commented Oct 22, 2021
View reviewed changes
tests/saiaclschema_ut.cpp Outdated Show resolved Hide resolved
bocon13
bocon13 commented Oct 22, 2021
View reviewed changes
tests/mocks/BUILD Outdated Show resolved Hide resolved
bocon13
bocon13 commented Oct 22, 2021
View reviewed changes
tests/fakes/BUILD Outdated Show resolved Hide resolved
bocon13
bocon13 commented Oct 22, 2021
View reviewed changes
tests/Makefile.am Outdated Show resolved Hide resolved
bocon13
bocon13 commented Oct 22, 2021
View reviewed changes
common/saiaclschema.h Outdated Show resolved Hide resolved
bocon13
bocon13 commented Oct 22, 2021
View reviewed changes
common/saiaclschema.cpp Outdated Show resolved Hide resolved
bocon13
bocon13 commented Oct 22, 2021
View reviewed changes
common/consumernotifier.h Outdated Show resolved Hide resolved
bocon13
bocon13 commented Oct 22, 2021
View reviewed changes
common/consumernotifier.cpp Outdated Show resolved Hide resolved
bocon13
bocon13 commented Oct 22, 2021
View reviewed changes
BUILD Outdated Show resolved Hide resolved
bocon13
bocon13 commented Oct 22, 2021
View reviewed changes
common/producerstatetable.h Outdated Show resolved Hide resolved
@bocon13 bocon13 force-pushed the pr2-batch-ops branch 2 times, most recently from a14f4d5 to 00a46d3 Compare October 22, 2021 16:36
mint570
mint570 reviewed Oct 22, 2021
View reviewed changes
Copy link

@mint570 mint570 left a comment

Choose a reason for hiding this comment

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

LGTM

@bocon13
Copy link
Author

bocon13 commented Oct 27, 2021

@qiluo-msft please take a look

@bocon13
Copy link
Author

bocon13 commented Oct 27, 2021 •
edited
Loading

@donNewtonAlpha Move this to upstream

@bocon13 bocon13 mentioned this pull request Oct 28, 2021
Merged
rhalstea
rhalstea reviewed Oct 28, 2021
View reviewed changes
Copy link

@rhalstea rhalstea left a comment

Choose a reason for hiding this comment

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

A lot of the fakes don't seem to be up to date with the more recent pins changes?

tests/fakes/fake_db_connector.cc Outdated

} // namespace

void FakeDBConnector::AddAppDbTable(const std::string& table_name,
Copy link

Choose a reason for hiding this comment

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

This should be named AddSonicDbTable.

Originally we only fakes AppDb tables, later we used the same paradigm to fake others.

@donNewtonAlpha donNewtonAlpha force-pushed the pr2-batch-ops branch 2 times, most recently from ce1b78a to 0596f17 Compare November 9, 2021 18:19
bocon13 and others added 2 commits November 9, 2021 23:25
@bocon13
Add ConsumerNotifier for PINS response path
7bca368 
* Add ConsumerNotifier for response path
* Introduce interfaces for DBConnector, NotificationConsumer,
  ConsumerNotifier, NotificationProducer, ProducerStateTable
  to make it easier to test these classes

Submission containing materials of a third party:
    Copyright Google LLC; Licensed under Apache 2.0

Co-authored-by: Akarsh Gupta <[email protected]>
Co-authored-by: Ashish Singh <[email protected]>
Co-authored-by: Jay Hu <[email protected]>
Co-authored-by: Manali Kumar <[email protected]>
Co-authored-by: Richard Yu <[email protected]>
Co-authored-by: Robert J. Halstead <[email protected]>
Co-authored-by: Runming Wu <[email protected]>
Co-authored-by: Srikishen Pondicherry Shanmugam <[email protected]>
Co-authored-by: Vivek Ramamoorthy <[email protected]>
Co-authored-by: Yilan Ji <[email protected]>

Signed-off-by: Don Newton [email protected]
@bocon13
Add new tests for notification classes
9eb5bc1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mint570 mint570 mint570 left review comments

@rhalstea rhalstea rhalstea left review comments

@prsunny prsunny Awaiting requested review from prsunny

@yxieca yxieca Awaiting requested review from yxieca

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bocon13 @rhalstea @mint570 @donNewtonAlpha