-
Notifications
You must be signed in to change notification settings - Fork 22
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
Schema checker improvements #395
Conversation
dan-mm
commented
Jul 23, 2023
- fixes: image is now mandatory in the case that there is no build key
- network key parsing should now accept keys and list as formats
- added new log-stdout, log-stderr, shell keys
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add tests for that. Especially with the network key we have run into issues before and a test is in dire need.
good call, tests added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These yaml files are not valid.
A key cannot be a duplicate. You cannot have the networks key twice, it will just be removed by the loader.
Please split the cases of having a list and a hash-map out into two tests.
To reproduce what I said::
>>> a = yaml.safe_load(open('/tmp/compose.yml'))
>>> a
{'name': 'Test', 'author': 'Dan Mateas', 'description': 'test', 'networks': ['network-a', 'network-b'], 'services': {'test-container': {'type': 'container', 'image': 'gcb_stress', 'build': '.'}}, 'flow': [{'name': 'Stress', 'container': 'test-container', 'commands': [{'type': 'console', 'command': 'stress-ng -c 1 -t 1 -q'}]}]}
As you see only one network key gets read.
…keys would not validate
@ArneTR good catch, thank you. Updated the tests. This also uncovered a case which I thought was working but was mistaken, where multiple network names as keys would not validate (it'd work only if there was one key). This has now been fixed in my last commit |
Eco-CI Output:
📈 Energy graph:
6.84 ┤ ╭╮ ╭──────────────╮ ╭──╮ ╭╮ ╭╮
6.35 ┤ ╭╮││ ╭╮ │ ╰╮ ╭───╯ │ ╭─╮ ╭──╯╰╮││
5.86 ┤ │╰╯│ ││ ╭╮ │ │ │ │ │ ╰╮ │ ╰╯│
5.37 ┤ │ │ ││ ╭╯│ ╭─╮ ╭╮ ╭────╮ ╭─╮ │ │╭╯ ╰╮ ╭─╯ │ ╭╯ │ ╭╮ ╭╮ ╭╮ ╭╮ ╭╮
4.88 ┤ │ ╰╮ │╰────────╮ ╭╮ ╭───╯ ╰─────╮╭───╮╭╮╭─╮╭╯ │ ╭╮ ╭────╮ │╰───╮ ╭╯ ╰────╯ │ ╭─╮╭──────╮│ ││ ╰────╮ ╭─────╮ │ ╰───╮│ │ ╭╮ │╰╮ ╭╮ ╭╮ ╭╮ ││ ││ ╭╮ ││ ╭╮ ╭╮ ││ ╭
4.39 ┤ ╭╮ ╭╮│ ╰───╯ ╰──╯╰──╯ ╰╯ ╰╯╰╯ ││ ╰─╯╰─╯ ╰─╮ │ │ │ ╰╮╭───╯ ╰╯ ││ ╰╯ │ │ ╰╮ │ ││ │ ││╭╮│ │ ╭╮ │╰╮ ╭╮ ││ ││ ╭╮ ╭╮ ╭╮ ││ ╭╮╭╮ ╭╮ ╭╮││ ││ ╭╮ ╭╮ ╭─╮ ╭╮ ╭─╮ ╭╮ ╭╮││ ╭╮ ╭╮ ╭╮ ││ ││╭╮ ╭╮ ╭╮ ╭╮││ ╭╮ ╭╯
3.90 ┤ │╰─╮│╰╯ ╰╯ ╰╮│ │ ╭╯ ╰╯ ││ │ │ │ │ ││ │ │╰╯╰╯ │ ╭╮╭╮ ││ │ │ ╭╮ ╭╯│ ││ ││ ││ ╭╮ ╭╮ │╰╮ ╭╮ ╭╮ ╭╮ ││ ╭─╯│ ╭╮ ╭╮ │╰╯│ ││ │╰╯│ ╭╮ ╭─╯│ ││╭╮ ││ │ ╰╮ ╭╮ ││ │ ╰╮ ╭╮ ││ │╰╯│ ╭╯│ ╭╮ │╰╮ ╭╯│ ││ ││││╭╮ ││ ╭─╮ ││ ╭╮ ╭╯╰╯│ ╭╯│ ╭╮╭╯
3.41 ┤ ╭╯ ╰╯ ││ │ │ ││ ╰─╯ ╰╮ │ ││ ╰╮ │ │ ╭╯│││ ││ │ │ │╰╮ │ │ │╰╮ │╰╮ ╭╯╰╮ │╰╮ ╭─╮ ││ │ │ ││ ││ │╰╮ ╭╯│ │ │ ││ ╭╯│ │ │ ││ │ │ ╭╮ │╰╮│ │ ╭╯│││ ││ │ │ ╭╯╰─╮ ││ │ │ ╭╯╰─╮ ││ │ │ │ │╭╮ ╭╯│╭╯ │ ╭╮ │ │ ╭╮ ╭╮ ╭╯╰╮ ╭╯╰╯│││ │╰╮ │ ╰╮ │╰╮ ╭╯╰─╮ │ │ │ │╭╮ ╭╯││
2.92 ┤ │ ╰╯ ╰──╯ ╰╯ ╰╮ ╭╮╭╯ ╰╯ │ │ ╰╮ │ ╰╯│ ╭╯│ │ │ │ │ │ │ │ │ ╭╯ │ │ │ │ │ ╭─╯ │ ╭╯╰╮ │ │ ╭╯╰╮ │╰╮ │ │ │ ╰╮ │ │ ││ │ │╭╯ │ ╭╮ ╭╮ │╰╮│ │ ││ │ ││ ╰╮ │ ╰╯│ ╭╯│╭╯ │ │ │ ╭╯│ │ │ │ ╰╮ │╰╮│ │ │ ╰╯│ │ ││ │ ╭╯│ │ │ ╭╯│ ╭╯│ │ │ ╭╯ │││ ╭╮ ╭╮ ╭╮│ │ ╭╯ │ ╭╮│ │ ╭╯ │ ╭╮ │ ╰╮ │ ╰╯│ │ ││
2.43 ┤ │ ╰─╯││ │ │ │ ╭╮ │ │ │ │╭╯ ╰╮ ╭╮ │ │ │ │ ╭╮╭╮ │ │ ╭╮ │ │ ╭╯ │ │ │ │ │ ╭╮ │ │ ╭╮ │ │ │ │ ╭╯ │ ╭╯ │ │ │ ╭╯ │ ╭╯│ ╭╮ │ ││ │ ╭╮ ╭╯╰╮ ││ │ ││ │ ││ ╭╮ │ ╰╯ │ │ │ ╭╮ │ ││ │ │ │ │ │╭╯ │ ╭╮ │ │ ╭╯ ││ │ │ │ │ ││ │ ╭╮╭╮ ╭╯ │ │ │╭─╮ │ │╭╮ │ │ ╭╯ │ │ ╰╯│ ││ ╭──╯╰─╯││ │ │ ╰╮ ╭╯││ │ ╭╯ │ ╭╯│╭───╯ │ │ │ │ ││
1.94 ┼────╯ ╰╯ ╰───────╯ ╰───╯╰──╯ ╰────────╯ ╰╯ ╰─────╯╰─╯ ╰────────╯ ╰─╯╰╯╰────╯ ╰─╯╰─────╯ ╰───────╯ ╰────────╯ ╰──────╯ ╰───╯╰───╯ ╰───╯╰───╯ ╰────────╯ ╰───────╯ ╰───────╯ ╰────────╯ ╰───────╯ ╰────────╯ ╰───╯╰───╯ ╰╯ ╰─╯╰────╯ ╰──╯╰────╯ ╰╯ ╰─────────╯╰───╯╰───╯ ╰───────╯ ╰──╯╰────╯ ╰╯ ╰───────╯ ╰────────╯ ╰╯ ╰──╯╰───╯ ╰───────╯ ╰╯ ╰────────╯ ╰────────╯ ╰╯ ╰──╯╰╯╰─╯ ╰────────╯ ╰╯ ╰─────╯ ╰╯╰──────╯ ╰───────╯ ╰───────╯ ╰──╯╰────╯ ╰╯ ╰───────╯ ╰────────╯ ╰╯ ╰─────╯ ╰───────╯ ╰╯ ╰───────╯ ╰────────╯ ╰╯
Watts over time |
changes made, unless there's anything else needed I think this can be merged in |