-
Notifications
You must be signed in to change notification settings - Fork 13
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
Enable VCL snippets #26
Conversation
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.
Tests are missing (see https://github.com/Jimdo/ansible-fastly/blob/master/tests/test_fastly_cache_settings.py how to test a new parameter/feature)
README.md
Outdated
```bash | ||
# prepare the environment ... | ||
$ printf '[defaults]\nroles_path=../' >ansible.cfg | ||
$ ln -snf $PWD ../Jimdo.fastly |
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.
I don't think this is necessary, the unit tests don't use ansible at all.
library/fastly_service.py
Outdated
@@ -327,6 +342,24 @@ def __init__(self, config, validate_choices): | |||
self.status = self.read_config(config, validate_choices, 'status') | |||
|
|||
|
|||
class FastlyVclSnippets(FastlyObject): |
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.
This should be FastlyVclSnippet
.
@fstehle you can take a look again, although i am not sure that setting up the services works correctly, because only one shows up in fastly, but when curling the API with the service ID's, i am getting the correct service back. It just doesn't show up in fastly. |
library/fastly_service.py
Outdated
@@ -107,6 +122,7 @@ | |||
|
|||
import httplib | |||
import urllib | |||
import json |
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.
This import is unused
|
||
active_version_number = service.active_version.number | ||
service = self.enforcer.apply_configuration(self.FASTLY_TEST_SERVICE, configuration).service | ||
self.assertEqual(service.active_version.number, active_version_number) |
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.
One test should be enough as these tests all test the same flow.
README.md
Outdated
``` | ||
FASTLY_API_KEY=some_secret python -m unittest discover tests | ||
# do it!! | ||
$ FASTLY_API_KEY=some_secret python -m unittest discover tests |
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.
What is this?
PTAL again @fstehle |
@fstehle @skrings PTAL
7 tests failed locally. they might fail on Travis, too. We'll see … :-|