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

Gennet/Kurtosis info into WLS with master changes #97

Merged
merged 118 commits into from
Mar 17, 2023

Conversation

AlbertoSoutullo
Copy link
Collaborator

@AlbertoSoutullo AlbertoSoutullo commented Mar 5, 2023

Another big PR, changes:

  • Using new nwaku image that uses base64 payloads
  • Changed wls.star to use base64 payloads
  • Analisys branch is already updated with latest master branch atm, so it was easier to do this refactor as it was very behind compared with master (half of the changes are because of this).
  • Now gennet topology and service information (like ip, ports and so on) is passed to WLS.
  • Refactored WLS module in several files to easier readability, maintainability and testing (can still be improved).
  • (bug) Nodes now correctly use --port-shift value when preparing ports in Kurtosis
  • (bug) Nodes now correctly use --port-shift value when preparing CMD in Kurtosis
  • (bug) Now WLS is correctly taking its configuration from config.json instead wls.yaml
  • (bug) Fixed mismatching nodes with topics. Topics were being selected from nodes that were not correspondant.
  • (bug) Arguments in WLS were not parsed, it was getting always default values.
  • (bug) Parameters in parser.add_argument were wrong (regarding action=store_true) making argparser not working correctly if arguments were passed.
  • (bug) uniform mode in wls was not working correctly in WLS. Fixed.
  • Cleaned unnused variables.
  • Added tests for WLS in python and Starlark (related with Test python modules #45 and Gennet: tests #68 ). Gennet is still missing tests.
  • Fixed old tests, now multinode approach is tested correctly.

Remember that in order to test the tests in starlark, you have to change the name of tests.star to main.star and do kurtosis run . while this process is not automatized with CI.

In order to run python tests, you can go to the desired directory (as wls-module) and run python -m unittest discover .

Closes #49 , closes #88 , closes #94 ,

@AlbertoSoutullo AlbertoSoutullo marked this pull request as ready for review March 13, 2023 15:44
@AlbertoSoutullo AlbertoSoutullo changed the base branch from Analysis to master March 13, 2023 15:45
@AlbertoSoutullo AlbertoSoutullo marked this pull request as draft March 13, 2023 15:46
@AlbertoSoutullo AlbertoSoutullo marked this pull request as ready for review March 13, 2023 18:35
Copy link
Contributor

@0xFugue 0xFugue left a comment

Choose a reason for hiding this comment

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

Glad we all switched to dispatchers! The WSL injection seems like a busy loop: if _time_to_send_next_message(...) is false, may be it is better to sleep for the remaining time as opposed to spin in while True. But LGTM otherwise.

@AlbertoSoutullo
Copy link
Collaborator Author

Glad we all switched to dispatchers! The WSL injection seems like a busy loop: if _time_to_send_next_message(...) is false, may be it is better to sleep for the remaining time as opposed to spin in while True. But LGTM otherwise.

This is indeed super good feedback and you are right. I would change it but we are planning to change this to non-blocking message inyection so it will be changed anyway.

@AlbertoSoutullo AlbertoSoutullo merged commit 4607a91 into master Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
4 participants