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.

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

Event and alarm management: Add tables for event and alarms. #852

Merged
merged 2 commits into from May 11, 2024

Conversation

bhaveshdell
Copy link
Contributor

Why I did it
This PR contains code changes for providing extension to the Event Framework as specified in the sonic-net/SONiC#1409

How I did it
Followed design specified in sonic-net/SONiC#1409.
Add new tables for events and alarms.

@shyam77git
Copy link

@bhaveshdell
This seems to be only 1 file change I see in this code PR.
Where are all other changes? PRs? could you please list them in HLD PR: sonic-net/SONiC#1409.

@bhaveshdell
Copy link
Contributor Author

bhaveshdell commented Jan 31, 2024

@bhaveshdell This seems to be only 1 file change I see in this code PR. Where are all other changes? PRs? could you please list them in HLD PR: sonic-net/SONiC#1409.

The PRs are listed in sonic-net/SONiC#1409

#852
sonic-net/sonic-buildimage#17949

@shyam77git
Copy link

@bhaveshdell This seems to be only 1 file change I see in this code PR. Where are all other changes? PRs? could you please list them in HLD PR: sonic-net/SONiC#1409.

The PRs are listed in sonic-net/SONiC#1409

#852 sonic-net/sonic-buildimage#17949

OK, so in all these two PRs to be reviewed? any more upcoming PR?
could you add me as reviewer to them both? Add others too so that review can be expedited

@bhaveshdell
Copy link
Contributor Author

@bhaveshdell This seems to be only 1 file change I see in this code PR. Where are all other changes? PRs? could you please list them in HLD PR: sonic-net/SONiC#1409.

The PRs are listed in sonic-net/SONiC#1409
#852 sonic-net/sonic-buildimage#17949

OK, so in all these two PRs to be reviewed? any more upcoming PR? could you add me as reviewer to them both? Add others too so that review can be expedited

@shyam77git Only these 2 PRs. Repo owner can add reviewer.

@bhaveshdell
Copy link
Contributor Author

/azpw run Azure.sonic-swss-common

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss-common

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@shyam77git
Copy link

@bhaveshdell This seems to be only 1 file change I see in this code PR. Where are all other changes? PRs? could you please list them in HLD PR: sonic-net/SONiC#1409.

The PRs are listed in sonic-net/SONiC#1409
#852 sonic-net/sonic-buildimage#17949

OK, so in all these two PRs to be reviewed? any more upcoming PR? could you add me as reviewer to them both? Add others too so that review can be expedited

@shyam77git Only these 2 PRs. Repo owner can add reviewer.

@prgeor / @zhangyanzhao - please add reviewers to this HLD.
This is a per-requisite for FM infra HLD: sonic-net/SONiC#1527

@bhaveshdell
Copy link
Contributor Author

/azpw run Azure.sonic-swss-common

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss-common

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bhaveshdell bhaveshdell changed the title Event and alarm management: Add newtables for event and alarms. Event and alarm management: Add tables for event and alarms. Feb 22, 2024
@jeff-yin
Copy link

@qiluo-msft @liuh-80 @shyam77git Please assign reviewers. Also, the build checks seem to be failing for reasons not related to the changes in this PR. Can you please check?

@bhaveshdell
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bhaveshdell
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bhaveshdell
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bhavini-gada
Copy link

@qiluo-msft @zbud-msft - Can we please get approval/merge on this PR?

@@ -23,6 +23,7 @@ namespace swss {
#define CHASSIS_APP_DB 12
#define CHASSIS_STATE_DB 13
#define APPL_STATE_DB 14
#define EVENT_DB 19
Copy link
Contributor

@qiluo-msft qiluo-msft May 7, 2024

Choose a reason for hiding this comment

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

There is some redis configuration

# Set the number of databases. The default database is DB 0, you can select
# a different one on a per-connection basis using SELECT <dbid> where
# dbid is a number between 0 and 'databases'-1
databases 16

If the max number is less than 19, will your feature work? Could you break the build prcoess if it happen? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current max number is set to 100 as part of this change
https://github.com/sonic-net/sonic-buildimage/pull/17161/files#diff-88ce9c28933f9dd9c3009adba8ae67827831c0d7b7ad20c72556ae18b3764931

This feature creates an instance of DB, and the above change to 100 suffices this features requirement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@qiluo-msft Please approve/merge this PR if no further comments.

@qiluo-msft qiluo-msft merged commit 0044540 into sonic-net:master May 11, 2024
17 checks passed
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.

None yet

7 participants