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

Make test_framework available for users when creating scenarios #619

Closed
wants to merge 10 commits into from

Conversation

pinheadmz
Copy link
Contributor

This changes two important things about how scenarios are written:

  1. Scenario files must be in a directory next to test_framework
  2. Scenario files must have a main() function, instead of (or in addition to) the __name__ == "__main__" logic:

example:

def main():
    ConnectDag().main()

Projects

When a user creates a project, they will get a scenarios/ directory which includes our default scenarios like miner_std.py as well as a complete copy of test_framework. They are welcome to edit the framework, the code in that directory is actually deployed.

Running Scenarios

When executing warnet run <path> this PR bundles up the scenario file, commander.py and the entire test_framework directory into a .pyz archive which is compressed, base64 encoded, and added as raw data in a config map that is deployed along with a vanilla, unmodified image of python:3.12-slim. Deploying our (current) biggest scenario signet_miner.py results in a config map of 118,459 bytes, which is still significantly below the 1 MB recommended maximum for configMap.

Side effects

The grossest thing about this PR in my opinion is that all the scenarios we use only for tests have to be moved as well, so they are in a directory with test_framework. To deal with that, I adjusted the "exclude" logic in copy_scenario_defaults() to filter by regex and then exclude TEST_*.py.

TODO

  • add a test that calls warnet init on an empty directory, runs a network and scenario from there to fully test the user flow (reccomend reviewers do this!)

@bdp-DrahtBot
Copy link
Collaborator

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #614 (swap out kubectl by mplsgrant)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

if any(
needle in str(path) for needle in [
".pyc",
".csv",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to categorically reject csv files? It's a common format, and I can imagine a user thinking they could use it.

@pinheadmz
Copy link
Contributor Author

fuck, right...
OSError: [Errno 7] Argument list too long: 'helm'
I guess a command line option with 100,000 bytes of base64 is overkill. Abandoning.

@pinheadmz pinheadmz closed this Sep 26, 2024
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.

3 participants