Skip to content

Conversation

@daall
Copy link
Contributor

@daall daall commented Aug 12, 2020

Signed-off-by: Danny Allen [email protected]

It is possible for changes in swss-common to have unexpected side effects that impact sairedis and swss. The best way to avoid merging such changes is to 1) build sairedis and swss against swss-common, and 2) run the virtual switch tests to make sure that swss-common is still operating as expected with the change.

This PR adds 1) and 2) to the build process for swss-common. This should help avoid merging breaking changes to the swss-common repo and keep the upstream PR tests behaving normally.

Blocked: discussions/testing ongoing.

@daall
Copy link
Contributor Author

daall commented Aug 13, 2020

@daall daall marked this pull request as draft August 13, 2020 01:24
@daall daall marked this pull request as ready for review August 13, 2020 22:59
@judyjoseph
Copy link

There is one point here .. So there are intermodular dependencies between sonic-swss and sonic-swss-common now. For eg: redefinition on DBConnector constructor in sonic-swss/tests/mock_tests/mock_dbconnector.cpp.
This could cause a deadlock in commit process into sonic-swss-common. This is because they share the common dbconnector.h file.
@daall We need to consider this before adding this PR build ?

@daall
Copy link
Contributor Author

daall commented Aug 14, 2020

There is one point here .. So there are intermodular dependencies between sonic-swss and sonic-swss-common now. For eg: redefinition on DBConnector constructor in sonic-swss/tests/mock_tests/mock_dbconnector.cpp.
This could cause a deadlock in commit process into sonic-swss-common. This is because they share the common dbconnector.h file.
@daall We need to consider this before adding this PR build ?

We do.

I added this check here so that Jenkins can publish the latest swss-common package without the VS tests passing 100%:

catchError(buildResult: 'SUCCESS', stageResult: 'UNSTABLE') {

I'm not sure if this is enough. Let's discuss more tomorrow before going forward with this.

@daall daall marked this pull request as draft August 14, 2020 21:36
@lguohan lguohan force-pushed the master branch 2 times, most recently from e187ae2 to 7f37a26 Compare December 26, 2020 11:59
@lguohan lguohan force-pushed the master branch 3 times, most recently from bde8e6c to 2295d0e Compare August 24, 2021 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants