fix(release controller): Only update posts if different and can_edit (#…

…391)

* fix(release controller): Only update posts if different and can_edit

Also update some related tests

* Some missing files

* some missing files and reviewer feedback applied

* Add back attributes required in tests

bin/poetry.sh

@@ -5,7 +5,7 @@ set -e
 git_root=$(git rev-parse --show-toplevel)
 (
     cd $git_root
-    bazel run //:poetry -- export -f requirements.txt > requirements.txt.2
+    bazel run //:poetry -- export --with dev -f requirements.txt >requirements.txt.2
     mv requirements.txt.2 requirements.txt
 )
 

pyproject.toml

@@ -56,6 +56,7 @@ wrapt = "1.16.0"
 
 [tool.poetry.group.dev.dependencies]
 black = "^24"
+httpretty = "^1.1.4"
 
 [build-system]
 requires = ["poetry-core"]

release-controller/BUILD.bazel

@@ -20,6 +20,10 @@ deps = [
     "//pylib",
 ]
 
+dev_deps = [
+    requirement("httpretty"),
+]
+
 env = {
     "BAZEL": "true",
 }
@@ -42,7 +46,7 @@ py_test(
     srcs = ["pytest.py"],
     data = glob(["*.py"]),
     env = env,
-    deps = deps,
+    deps = deps + dev_deps,
 )
 
 py_oci_image(

release-controller/forum.py

@@ -1,3 +1,4 @@
+import logging
 import os
 from typing import Callable
 
@@ -82,10 +83,20 @@ def update(self, changelog: Callable[[str], str | None], proposal: Callable[[str
         created_posts = self.created_posts()
         for i, p in enumerate(posts):
             if i < len(created_posts):
-                self.client.update_post(
-                    post_id=created_posts[i]["id"],
-                    content=_post_template(version_name=p.version_name, changelog=p.changelog, proposal=p.proposal),
+                post_id = created_posts[i]["id"]
+                content_expected = _post_template(
+                    version_name=p.version_name, changelog=p.changelog, proposal=p.proposal
                 )
+                post = self.client.post_by_id(post_id)
+                if post["raw"] == content_expected:
+                    # log the complete URL of the post
+                    logging.info("post up to date: %s", self.post_to_url(post))
+                    continue
+                elif post["can_edit"]:
+                    logging.info("updating post %s", post_id)
+                    self.client.update_post(post_id=post_id, content=content_expected)
+                else:
+                    logging.error("cannot update post %s", post_id)
             else:
                 self.client.create_post(
                     topic_id=self.topic_id,
@@ -98,7 +109,10 @@ def post_url(self, version: str):
         post = self.client.post_by_id(post_id=self.created_posts()[post_index]["id"])
         if not post:
             raise RuntimeError("failed to find post")
+        return self.post_to_url(post)
 
+    def post_to_url(self, post: dict):
+        """Return the complete URL of the given post."""
         host = self.client.host.removesuffix("/")
         return f"{host}/t/{post['topic_slug']}/{post['topic_id']}/{post['post_number']}"
 

release-controller/mock_discourse.py

@@ -2,12 +2,19 @@
 
 
 class DiscourseClientMock(DiscourseClient):
+    """A mock Discourse client."""
+
     def __init__(self):
+        """Create a new mock client. The actual host needs to be mocked in the test."""
+        self.host = "http://localhost:55555/"
         self.created_topics = []
         self.created_posts = []
         self.api_username = "test"
+        self.api_key = "test_api_key"
+        self.timeout = 10
 
     def categories(self):
+        """Return a list of categories."""
         return [
             {"id": i} | t
             for i, t in enumerate(
@@ -21,9 +28,11 @@ def categories(self):
         ]
 
     def topics_by(self, _: str):
+        """Return a list of topics."""
         return [{"id": i + 1} | t for i, t in enumerate(self.created_topics)]
 
     def topic_posts(self, topic_id: str):
+        """Return a list of posts in a topic."""
         return {
             "post_stream": {
                 "posts": [
@@ -40,20 +49,23 @@ def create_post(
         category_id=None,
         topic_id=None,
         title=None,
-        tags=[],
-        **kwargs,
+        tags=None,
+        **kwargs,  # pylint: disable=unused-argument
     ):
+        """Create a new post. If topic_id is not provided, a new topic is created."""
         if not topic_id:
-            self.created_topics.append({"title": title, "category_id": category_id, "tags": tags})
+            self.created_topics.append({"title": title, "category_id": category_id, "tags": tags or []})
             topic_id = self.topics_by("")[-1]["id"]
         self.created_posts.append(
             {
                 "raw": content,
                 "topic_id": topic_id,
                 "yours": True,
+                "can_edit": True,
             }
         )
         return self.topic_posts(topic_id=topic_id)["post_stream"]["posts"][-1]
 
-    def update_post(self, post_id, content, edit_reason="", **kwargs):
+    def update_post(self, post_id, content, edit_reason="", **kwargs):  # pylint: disable=unused-argument
+        """Update an existing post."""
         self.created_posts[post_id - 1]["raw"] = content

release-controller/release_index.py

@@ -1,57 +1,61 @@
 # generated by datamodel-codegen:
 #   filename:  release-index-schema.json
-
 from __future__ import annotations
 
 from datetime import date
-from typing import List, Optional
+from typing import List
+from typing import Optional
 
-from pydantic import BaseModel, ConfigDict, RootModel
+from pydantic import BaseModel
+from pydantic import ConfigDict
+from pydantic import RootModel
 
 
 class Version(BaseModel):
-    model_config = ConfigDict(
-        extra='forbid',
-    )
+    """A version of the release."""
+
+    model_config = ConfigDict(extra="forbid")
     version: str
     name: str
     subnets: Optional[List[str]] = None
 
 
 class Stage(BaseModel):
-    model_config = ConfigDict(
-        extra='forbid',
-    )
+    """A stage in the rollout."""
+
+    model_config = ConfigDict(extra="forbid")
     subnets: Optional[List[str]] = None
     bake_time: Optional[str] = None
     update_unassigned_nodes: Optional[bool] = None
     wait_for_next_week: Optional[bool] = None
 
 
 class Release(BaseModel):
-    model_config = ConfigDict(
-        extra='forbid',
-    )
+    """A release."""
+
+    model_config = ConfigDict(extra="forbid")
     rc_name: str
     versions: List[Version]
 
 
 class Rollout(BaseModel):
-    model_config = ConfigDict(
-        extra='forbid',
-    )
+    """A rollout."""
+
+    model_config = ConfigDict(extra="forbid")
     pause: Optional[bool] = None
     skip_days: Optional[List[date]] = None
     stages: List[Stage]
 
 
 class ReleaseIndex(BaseModel):
-    model_config = ConfigDict(
-        extra='forbid',
-    )
+    """The release index."""
+
+    model_config = ConfigDict(extra="forbid")
     rollout: Rollout
     releases: List[Release]
 
 
 class Model(RootModel[ReleaseIndex]):
+    """The root model."""
+
     root: ReleaseIndex

release-controller/test_forum.py

@@ -1,15 +1,21 @@
-import pytest
-from forum import ReleaseCandidateForumClient, ReleaseCandidateForumPost
+import httpretty.utils
+from forum import ReleaseCandidateForumClient
 from mock_discourse import DiscourseClientMock
-from release_index import Release, Version
+from release_index import Release
+from release_index import Version
 
 
+@httpretty.activate(verbose=True, allow_net_connect=False)
 def test_create_release_notes_on_new_release():
-    """
-    Test that when the new release is added to the index, reconciler creates release notes for engineers to edit
-    """
-
+    """Release notes are created when a new release is added to the index."""
     discourse_client = DiscourseClientMock()
+    get_url = discourse_client.host + "/posts/1.json"
+    httpretty.register_uri(
+        httpretty.GET,
+        get_url,
+        body='{"raw": "bogus text", "can_edit": true}',
+        content_type="application/json; charset=utf-8",
+    )
     assert discourse_client.created_posts == []
     assert discourse_client.created_topics == []
     forum_client = ReleaseCandidateForumClient(discourse_client=discourse_client)
@@ -30,9 +36,8 @@ def proposal(v: str):
         return int(v.removeprefix("test"))
 
     post.update(changelog=changelog, proposal=proposal)
-    assert discourse_client.created_posts == [
-        {
-            "raw": """\
+    expected_post_1 = {
+        "raw": """\
 Hello there!
 
 We are happy to announce that voting is now open for [a new IC release](https://github.com/dfinity/ic/tree/release-2024-02-21_23-06-default).
@@ -42,11 +47,12 @@ def proposal(v: str):
 
 release notes for version test1...
 """,
-            "yours": True,
-            "topic_id": 1,
-        },
-        {
-            "raw": """\
+        "yours": True,
+        "topic_id": 1,
+        "can_edit": True,
+    }
+    expected_post_2 = {
+        "raw": """\
 Hello there!
 
 We are happy to announce that voting is now open for [a new IC release](https://github.com/dfinity/ic/tree/release-2024-02-21_23-06-feat).
@@ -56,10 +62,11 @@ def proposal(v: str):
 
 release notes for version test2...
 """,
-            "yours": True,
-            "topic_id": 1,
-        },
-    ]
+        "yours": True,
+        "topic_id": 1,
+        "can_edit": True,
+    }
+    assert discourse_client.created_posts == [expected_post_1, expected_post_2]
 
     assert discourse_client.created_topics == [
         {

requirements.txt

@@ -98,7 +98,7 @@ bcrypt==4.1.2 ; python_full_version >= "3.10.0" and python_version < "4" \
 beautifulsoup4==4.12.3 ; python_full_version >= "3.10.0" and python_version < "4" \
     --hash=sha256:74e3d1928edc070d21748185c46e3fb33490f22f52a3addee9aee0f4f7781051 \
     --hash=sha256:b80878c9f40111313e55da8ba20bdba06d8fa3969fc68304167741bbf9e082ed
-black==24.3.0 ; python_full_version >= "3.10.0" and python_version < "4.0" \
+black==24.3.0 ; python_full_version >= "3.10.0" and python_version < "4" \
     --hash=sha256:2818cf72dfd5d289e48f37ccfa08b460bf469e67fb7c4abb07edc2e9f16fb63f \
     --hash=sha256:41622020d7120e01d377f74249e677039d20e6344ff5851de8a10f11f513bf93 \
     --hash=sha256:4acf672def7eb1725f41f38bf6bf425c8237248bb0804faa3965c036f7672d11 \
@@ -714,6 +714,8 @@ httpcore==1.0.5 ; python_full_version >= "3.10.0" and python_version < "4" \
 httplib2==0.22.0 ; python_full_version >= "3.10.0" and python_version < "4" \
     --hash=sha256:14ae0a53c1ba8f3d37e9e27cf37eabb0fb9980f435ba405d546948b009dd64dc \
     --hash=sha256:d7a10bc5ef5ab08322488bde8c726eeee5c8618723fdb399597ec58f3d82df81
+httpretty==1.1.4 ; python_full_version >= "3.10.0" and python_version < "4" \
+    --hash=sha256:20de0e5dd5a18292d36d928cc3d6e52f8b2ac73daec40d41eb62dee154933b68
 httpx==0.27.0 ; python_full_version >= "3.10.0" and python_version < "4" \
     --hash=sha256:71d5465162c13681bff01ad59b2cc68dd838ea1f10e51574bac27103f00c91a5 \
     --hash=sha256:a0cb88a46f32dc874e04ee956e4c2764aba2aa228f650b06788ba6bda2962ab5
@@ -1112,6 +1114,7 @@ msgpack==1.0.8 ; python_full_version >= "3.10.0" and python_version < "4.0" \
     --hash=sha256:1876b0b653a808fcd50123b953af170c535027bf1d053b59790eebb0aeb38950 \
     --hash=sha256:1ab0bbcd4d1f7b6991ee7c753655b481c50084294218de69365f8f1970d4c151 \
     --hash=sha256:1cce488457370ffd1f953846f82323cb6b2ad2190987cd4d70b2713e17268d24 \
+    --hash=sha256:24f727df1e20b9876fa6e95f840a2a2651e34c0ad147676356f4bf5fbb0206ca \
     --hash=sha256:26ee97a8261e6e35885c2ecd2fd4a6d38252246f94a2aec23665a4e66d066305 \
     --hash=sha256:3528807cbbb7f315bb81959d5961855e7ba52aa60a3097151cb21956fbc7502b \
     --hash=sha256:374a8e88ddab84b9ada695d255679fb99c53513c0a51778796fcf0944d6c789c \
@@ -1163,7 +1166,7 @@ msgpack==1.0.8 ; python_full_version >= "3.10.0" and python_version < "4.0" \
 multimethod==1.11.2 ; python_full_version >= "3.10.0" and python_version < "4" \
     --hash=sha256:7f2a4863967142e6db68632fef9cd79053c09670ba0c5f113301e245140bba5c \
     --hash=sha256:cb338f09395c0ee87d36c7691cdd794d13d8864358082cf1205f812edd5ce05a
-mypy-extensions==1.0.0 ; python_full_version >= "3.10.0" and python_version < "4.0" \
+mypy-extensions==1.0.0 ; python_full_version >= "3.10.0" and python_version < "4" \
     --hash=sha256:4392f6c0eb8a5668a69e23d168ffa70f0be9ccfd32b5cc2d26a34ae5b844552d \
     --hash=sha256:75dbf8955dc00442a438fc4d0666508a9a97b6bd41aa2f0ffe9d2f2725af0782
 nbclient==0.10.0 ; python_full_version >= "3.10.0" and python_version < "4" \
@@ -1638,7 +1641,6 @@ pyyaml==6.0.1 ; python_version >= "3.10" and python_version < "4" \
     --hash=sha256:8d4e9c88387b0f5c7d5f281e55304de64cf7f9c0021a3525bd3b1c542da3b0e4 \
     --hash=sha256:9046c58c4395dff28dd494285c82ba00b546adfc7ef001486fbf0324bc174fba \
     --hash=sha256:9eb6caa9a297fc2c2fb8862bc5370d0303ddba53ba97e71f08023b6cd73d16a8 \
-    --hash=sha256:a08c6f0fe150303c1c6b71ebcd7213c2858041a7e01975da3a99aed1e7a378ef \
     --hash=sha256:a0cd17c15d3bb3fa06978b4e8958dcdc6e0174ccea823003a106c7d4d7899ac5 \
     --hash=sha256:afd7e57eddb1a54f0f1a974bc4391af8bcce0b444685d936840f125cf046d5bd \
     --hash=sha256:b1275ad35a5d18c62a7220633c913e1b42d44b46ee12554e5fd39c70a243d6a3 \