Skip to content

Commit

Permalink
Address @mythmon comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@leplatrem
leplatrem committed Mar 17, 2020
1 parent aa1ef0c commit 28e2252
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 43 deletions.
2 changes: 1 addition & 1 deletion docs/ops/config.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ in other Django projects.

The account password to authenticate as DJANGO_REMOTE_SETTINGS_USERNAME.

.. envvar:: REMOTE_SETTINGS_CAPABILITIES_COLLECTION_ID
.. envvar:: DJANGO_REMOTE_SETTINGS_CAPABILITIES_COLLECTION_ID

:default: ``normandy-recipes-capabilities``

Expand Down
18 changes: 9 additions & 9 deletions normandy/recipes/tests/test_commands.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,13 +495,13 @@ def test_publishes_missing_recipes(self, rs_settings, requestsmock):
call_command("sync_remote_settings")

requests = requestsmock.request_history
# The first two requests should be to get the existing records
# First request should be to get the existing records
assert requests[0].method == "GET"
assert requests[0].url.endswith(self.capabilities_published_records_url)
# The next two should be to PUT the missing recipe2
# The next should be to PUT the missing recipe2
assert requests[1].method == "PUT"
assert requests[1].url.endswith(r2_capabilities_url)
# The final two should be to approve the changes to the two collections
# The final one should be to approve the changes
assert requests[2].method == "PATCH"
assert requests[2].url.endswith(self.capabilities_workspace_collection_url)
# And there are no extra requests
Expand Down Expand Up @@ -531,13 +531,13 @@ def test_republishes_outdated_recipes(self, rs_settings, requestsmock):
call_command("sync_remote_settings")

requests = requestsmock.request_history
# The first two requests should be to get the existing records
# The first request should be to get the existing records
assert requests[0].method == "GET"
assert requests[0].url.endswith(self.capabilities_published_records_url)
# The next two should be to PUT the outdated recipe2
# The next one should be to PUT the outdated recipe2
assert requests[1].method == "PUT"
assert requests[1].url.endswith(r2_capabilities_url)
# The final two should be to approve the changes to the two collections
# The final one should be to approve the changes
assert requests[2].method == "PATCH"
assert requests[2].url.endswith(self.capabilities_workspace_collection_url)
# And there are no extra requests
Expand Down Expand Up @@ -566,13 +566,13 @@ def test_unpublishes_extra_recipes(self, rs_settings, requestsmock):
call_command("sync_remote_settings")

requests = requestsmock.request_history
# The first two requests should be to get the existing records
# The first request should be to get the existing records
assert requests[0].method == "GET"
assert requests[0].url.endswith(self.capabilities_published_records_url)
# The next two should be to PUT the outdated recipe2
# The next one should be to PUT the outdated recipe2
assert requests[1].method == "DELETE"
assert requests[1].url.endswith(r2_capabilities_url)
# The final two should be to approve the changes to the two collections
# The final one should be to approve the changes
assert requests[2].method == "PATCH"
assert requests[2].url.endswith(self.capabilities_workspace_collection_url)
# And there are no extra requests
Expand Down
56 changes: 23 additions & 33 deletions normandy/recipes/tests/test_exports.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,21 @@ def mock_logger(mocker):
class TestRemoteSettings:
@pytest.fixture
def rs_urls(self, rs_settings):
rv = {
"workspace": {"capabilities": {}, "baseline": {}},
"publish": {"capabilities": {}, "baseline": {}},
}
rv["workspace"]["capabilities"]["collection"] = (
rv = {"workspace": {}, "publish": {}}
rv["workspace"]["collection"] = (
f"{rs_settings.REMOTE_SETTINGS_URL}"
f"/buckets/{rs_settings.REMOTE_SETTINGS_WORKSPACE_BUCKET_ID}"
f"/collections/{rs_settings.REMOTE_SETTINGS_CAPABILITIES_COLLECTION_ID}"
)
rv["publish"]["capabilities"]["collection"] = (
rv["publish"]["collection"] = (
f"{rs_settings.REMOTE_SETTINGS_URL}"
f"/buckets/{rs_settings.REMOTE_SETTINGS_PUBLISH_BUCKET_ID}"
f"/collections/{rs_settings.REMOTE_SETTINGS_CAPABILITIES_COLLECTION_ID}"
)

for bucket in ["workspace", "publish"]:
rv[bucket]["capabilities"]["records"] = (
rv[bucket]["capabilities"]["collection"] + "/records"
)
rv[bucket]["capabilities"]["record"] = (
rv[bucket]["capabilities"]["collection"] + "/records/{}"
)
rv[bucket]["records"] = rv[bucket]["collection"] + "/records"
rv[bucket]["record"] = rv[bucket]["collection"] + "/records/{}"

return rv

Expand Down Expand Up @@ -277,13 +270,13 @@ def test_publish_puts_record_and_approves(

requestsmock.request(
"PUT",
rs_urls["workspace"]["capabilities"]["record"].format(recipe.id),
rs_urls["workspace"]["record"].format(recipe.id),
json={"data": {}},
request_headers=request_headers,
)
requestsmock.request(
"PATCH",
rs_urls["workspace"]["capabilities"]["collection"],
rs_urls["workspace"]["collection"],
json={"data": {}},
request_headers=request_headers,
)
Expand All @@ -293,11 +286,11 @@ def test_publish_puts_record_and_approves(

requests = requestsmock.request_history
assert len(requests) == 2
assert requests[0].url == rs_urls["workspace"]["capabilities"]["record"].format(recipe.id)
assert requests[0].url == rs_urls["workspace"]["record"].format(recipe.id)
assert requests[0].method == "PUT"
assert requests[0].json() == {"data": exports.recipe_as_record(recipe)}
assert requests[1].method == "PATCH"
assert requests[1].url == rs_urls["workspace"]["capabilities"]["collection"]
assert requests[1].url == rs_urls["workspace"]["collection"]
mock_logger.info.assert_called_with(
f"Published record '{recipe.id}' for recipe '{recipe.approved_revision.name}'"
)
Expand All @@ -320,33 +313,30 @@ def test_unpublish_deletes_record_and_approves(
}
requestsmock.request(
"delete",
urls["capabilities"]["record"].format(recipe.id),
urls["record"].format(recipe.id),
json={"data": {}},
request_headers=request_headers,
)
requestsmock.request(
"patch",
urls["capabilities"]["collection"],
json={"data": {}},
request_headers=request_headers,
"patch", urls["collection"], json={"data": {}}, request_headers=request_headers
)

remotesettings = exports.RemoteSettings()
remotesettings.unpublish(recipe)

assert len(requestsmock.request_history) == 2
requests = requestsmock.request_history
assert requests[0].url == urls["capabilities"]["record"].format(recipe.id)
assert requests[0].url == urls["record"].format(recipe.id)
assert requests[0].method == "DELETE"
assert requests[1].url == urls["capabilities"]["collection"]
assert requests[1].url == urls["collection"]
assert requests[1].method == "PATCH"
mock_logger.info.assert_called_with(
f"Deleted record '{recipe.id}' of recipe '{recipe.approved_revision.name}'"
)

def test_publish_raises_an_error_if_request_fails(self, rs_urls, rs_settings, requestsmock):
recipe = RecipeFactory(name="Test", approver=UserFactory())
record_url = rs_urls["workspace"]["capabilities"]["record"].format(recipe.id)
record_url = rs_urls["workspace"]["record"].format(recipe.id)
requestsmock.request("put", record_url, status_code=503)

remotesettings = exports.RemoteSettings()
Expand All @@ -359,7 +349,7 @@ def test_unpublish_ignores_error_about_missing_records(
self, rs_urls, rs_settings, requestsmock, mock_logger
):
recipe = RecipeFactory(name="Test", approver=UserFactory())
capabilities_record_url = rs_urls["workspace"]["capabilities"]["record"].format(recipe.id)
capabilities_record_url = rs_urls["workspace"]["record"].format(recipe.id)
warning_message = (
f"The recipe '{recipe.id}' was not published in the {{}} collection. Skip."
)
Expand All @@ -375,12 +365,12 @@ def test_publish_reverts_changes_if_approval_fails(self, rs_urls, rs_settings, r
# simplify the test. This simplifies the test.
recipe = RecipeFactory(name="Test", approver=UserFactory())

capabilities_record_url = rs_urls["workspace"]["capabilities"]["record"].format(recipe.id)
capabilities_record_url = rs_urls["workspace"]["record"].format(recipe.id)
# Creating the record works.
requestsmock.request("put", capabilities_record_url, json={"data": {}}, status_code=201)
requestsmock.register_uri(
"patch",
rs_urls["workspace"]["capabilities"]["collection"],
rs_urls["workspace"]["collection"],
[
# Approving fails.
{"status_code": 403},
Expand All @@ -400,20 +390,20 @@ def test_publish_reverts_changes_if_approval_fails(self, rs_urls, rs_settings, r
assert requests[0].url == capabilities_record_url
# and then tries to approve it, which fails.
assert requests[1].method == "PATCH"
assert requests[1].url == rs_urls["workspace"]["capabilities"]["collection"]
assert requests[1].url == rs_urls["workspace"]["collection"]
# so it rollsback
assert requests[2].method == "PATCH"
assert requests[2].url == rs_urls["workspace"]["capabilities"]["collection"]
assert requests[2].url == rs_urls["workspace"]["collection"]

def test_unpublish_reverts_changes_if_approval_fails(self, rs_urls, rs_settings, requestsmock):
recipe = RecipeFactory(name="Test", approver=UserFactory())
capabilities_record_url = rs_urls["workspace"]["capabilities"]["record"].format(recipe.id)
capabilities_record_url = rs_urls["workspace"]["record"].format(recipe.id)
# Deleting the record works.
requestsmock.request("delete", capabilities_record_url, json={"data": {"deleted": True}})

requestsmock.register_uri(
"patch",
rs_urls["workspace"]["capabilities"]["collection"],
rs_urls["workspace"]["collection"],
[
# Approving fails.
{"status_code": 403},
Expand All @@ -432,8 +422,8 @@ def test_unpublish_reverts_changes_if_approval_fails(self, rs_urls, rs_settings,
assert requests[0].url == capabilities_record_url
assert requests[0].method == "DELETE"
# Try (and fail) to approve the capabilities change
assert requests[1].url == rs_urls["workspace"]["capabilities"]["collection"]
assert requests[1].url == rs_urls["workspace"]["collection"]
assert requests[1].method == "PATCH"
# so it rollsback collection
assert requests[2].method == "PATCH"
assert requests[2].url == rs_urls["workspace"]["capabilities"]["collection"]
assert requests[2].url == rs_urls["workspace"]["collection"]

0 comments on commit 28e2252

Please sign in to comment.