From be1882c3f3f87551b50b3201c71f953e10dfb452 Mon Sep 17 00:00:00 2001 From: nikolamilosa Date: Wed, 9 Oct 2024 01:45:55 +0200 Subject: [PATCH 1/4] using different api for pagination --- release-controller/forum.py | 78 +++++++++++++----------- release-controller/reconciler.py | 85 +++++++-------------------- release-controller/test_forum.py | 8 ++- release-controller/test_reconciler.py | 7 ++- 4 files changed, 77 insertions(+), 101 deletions(-) diff --git a/release-controller/forum.py b/release-controller/forum.py index 9f4e6294d..de318ef43 100644 --- a/release-controller/forum.py +++ b/release-controller/forum.py @@ -1,8 +1,8 @@ -import json import logging import os from typing import Callable +import requests from dotenv import load_dotenv from pydantic_yaml import parse_yaml_raw_as from pydiscourse import DiscourseClient @@ -45,17 +45,19 @@ def __init__( release: Release, client: DiscourseClient, nns_proposal_discussions_category, + discourse_url: str, + discourse_username: str, + discourse_api_key: str, ): """Create a new topic.""" self.release = release self.client = client self.nns_proposal_discussions_category = nns_proposal_discussions_category + self.discourse_url = discourse_url + self.discourse_api_key = discourse_api_key + self.discourse_username = discourse_username topic = next( - ( - t - for t in client.topics_by(self.client.api_username) - if self.release.rc_name in t["title"] - ), + (t for t in client.topics_by(self.client.api_username) if self.release.rc_name in t["title"]), None, ) if topic: @@ -75,12 +77,15 @@ def __init__( def created_posts(self): """Return a list of posts created by the current user.""" topic_posts = self.client.topic_posts(topic_id=self.topic_id) + topic_posts = requests.get( + f"{self.discourse_url}/t/{self.topic_id}.json?print=true", + headers={"Api-Username": self.discourse_username, "Api-Key": self.discourse_api_key}, + ).json() + if not topic_posts: raise RuntimeError("failed to list topic posts") - return [ - p for p in topic_posts.get("post_stream", {}).get("posts", {}) if p["yours"] - ] + return [p for p in topic_posts.get("post_stream", {}).get("posts", {}) if p["yours"]] def update( self, @@ -128,9 +133,7 @@ def update( def post_url(self, version: str): """Return the URL of the post for the given version.""" - post_index = [ - i for i, v in enumerate(self.release.versions) if v.version == version - ][0] + post_index = [i for i, v in enumerate(self.release.versions) if v.version == version][0] post = self.client.post_by_id(post_id=self.created_posts()[post_index]["id"]) if not post: raise RuntimeError("failed to find post") @@ -152,7 +155,9 @@ def add_version(self, content: str): class ReleaseCandidateForumClient: """A client for interacting with release candidate forum topics.""" - def __init__(self, discourse_client: DiscourseClient): + def __init__( + self, discourse_client: DiscourseClient, discourse_url: str, discourse_username: str, discourse_api_key: str + ): """Create a new client.""" self.discourse_client = discourse_client self.nns_proposal_discussions_category = next( @@ -165,6 +170,9 @@ def __init__(self, discourse_client: DiscourseClient): "category" ], # hardcoded category id, seems like "include_subcategories" is not working ) + self.discourse_url = discourse_url + self.discourse_username = discourse_username + self.discourse_api_key = discourse_api_key def get_or_create(self, release: Release) -> ReleaseCandidateForumTopic: """Get or create a forum topic for the given release.""" @@ -172,6 +180,9 @@ def get_or_create(self, release: Release) -> ReleaseCandidateForumTopic: release=release, client=self.discourse_client, nns_proposal_discussions_category=self.nns_proposal_discussions_category, + discourse_url=self.discourse_url, + discourse_username=self.discourse_username, + discourse_api_key=self.discourse_api_key, ) @@ -183,32 +194,31 @@ def main(): api_username=os.environ["DISCOURSE_USER"], api_key=os.environ["DISCOURSE_KEY"], ) - # index = parse_yaml_raw_as( - # Model, - # """ - # rollout: - # stages: [] - - # releases: - # - rc_name: rc--2024-03-13_23-05 - # versions: - # - version: 2e921c9adfc71f3edc96a9eb5d85fc742e7d8a9f - # name: default - # - version: 31e9076fb99dfc36eb27fb3a2edc68885e6163ac - # name: feat - # - version: db583db46f0894d35bcbcfdea452d93abdadd8a6 - # name: feat-hotfix1 - # """, - # ) + index = parse_yaml_raw_as( + Model, + """ + releases: + - rc_name: rc--2024-10-03_01-30 + versions: + - name: base + version: d2657773d007e1b4c0b2dd715c628d24c0d7b5fb + - name: revert-ubuntu-22-04 + version: 1ff0e709f0d0984a4f9ab06456db177c4b6e48a0 + - name: canister-overhead-hotfix + version: f0c923eba09e9c1444501692b0ab4884882bf5bc + """, + ) forum_client = ReleaseCandidateForumClient( discourse_client, + discourse_url=os.environ["DISCOURSE_URL"], + discourse_api_key=os.environ["DISCOURSE_KEY"], + discourse_username=os.environ["DISCOURSE_USER"], ) + topic = forum_client.get_or_create(index.root.releases[0]) + # topic.update(lambda _: None, lambda _: None) -# topic = forum_client.get_or_create(index.root.releases[0]) -# topic.update(lambda _: None, lambda _: None) - -# print(topic.post_url(version="31e9076fb99dfc36eb27fb3a2edc68885e6163ac")) + print(topic.post_url(version="f0c923eba09e9c1444501692b0ab4884882bf5bc")) if __name__ == "__main__": diff --git a/release-controller/reconciler.py b/release-controller/reconciler.py index d0fb26985..f6fa30e08 100644 --- a/release-controller/reconciler.py +++ b/release-controller/reconciler.py @@ -9,6 +9,7 @@ import time import traceback import typing + import dre_cli @@ -62,14 +63,10 @@ def proposal_submitted(self, version: str) -> bool: if self._version_path(version).exists(): proposal_id = self.version_proposal(version) if proposal_id: - logging.info( - "version %s: proposal %s already submitted", version, proposal_id - ) + logging.info("version %s: proposal %s already submitted", version, proposal_id) else: last_modified = datetime.datetime.fromtimestamp(os.path.getmtime(version_path)) - remaining_time_until_retry = datetime.timedelta(minutes=10) - ( - datetime.datetime.now() - last_modified - ) + remaining_time_until_retry = datetime.timedelta(minutes=10) - (datetime.datetime.now() - last_modified) if remaining_time_until_retry.total_seconds() > 0: logging.warning( "version %s: earlier proposal submission attempted but most likely failed, will retry in %s seconds", @@ -94,9 +91,7 @@ def save_proposal(self, version: str, proposal_id: int): f.write(str(proposal_id)) -def oldest_active_release( - index: release_index.Model, active_versions: list[str] -) -> release_index.Release: +def oldest_active_release(index: release_index.Model, active_versions: list[str]) -> release_index.Release: for rc in reversed(index.root.releases): for v in rc.versions: if v.version in active_versions: @@ -109,36 +104,20 @@ def versions_to_unelect( index: release_index.Model, active_versions: list[str], elected_versions: list[str] ) -> list[str]: active_releases_versions = [] - for rc in index.root.releases[ - : index.root.releases.index(oldest_active_release(index, active_versions)) + 1 - ]: + for rc in index.root.releases[: index.root.releases.index(oldest_active_release(index, active_versions)) + 1]: for v in rc.versions: active_releases_versions.append(v.version) - return [ - v - for v in elected_versions - if v not in active_releases_versions and v not in active_versions - ] + return [v for v in elected_versions if v not in active_releases_versions and v not in active_versions] -def find_base_release( - ic_repo: GitRepo, config: release_index.Model, commit: str -) -> typing.Tuple[str, str]: - """ - Find the parent release commit for the given commit. Optionally return merge base if it's not a direct parent. - """ +def find_base_release(ic_repo: GitRepo, config: release_index.Model, commit: str) -> typing.Tuple[str, str]: + """Find the parent release commit for the given commit. Optionally return merge base if it's not a direct parent.""" ic_repo.fetch() rc, rc_idx = next( - (rc, i) - for i, rc in enumerate(config.root.releases) - if any(v.version == commit for v in rc.versions) - ) - v_idx = next( - i - for i, v in enumerate(config.root.releases[rc_idx].versions) - if v.version == commit + (rc, i) for i, rc in enumerate(config.root.releases) if any(v.version == commit for v in rc.versions) ) + v_idx = next(i for i, v in enumerate(config.root.releases[rc_idx].versions) if v.version == commit) return ( ( config.root.releases[rc_idx + 1].versions[0].version, @@ -149,11 +128,7 @@ def find_base_release( ) # take first version from the previous rc if v_idx == 0 else min( - [ - (v.version, version_name(rc.rc_name, v.name)) - for v in rc.versions - if v.version != commit - ], + [(v.version, version_name(rc.rc_name, v.name)) for v in rc.versions if v.version != commit], key=lambda v: ic_repo.distance(ic_repo.merge_base(v[0], commit), commit), ) ) @@ -223,9 +198,7 @@ def __init__( self.nns_url = nns_url self.governance_canister = GovernanceCanister() self.state = state - self.ic_prometheus = ICPrometheus( - url="https://victoria.mainnet.dfinity.network/select/0/prometheus" - ) + self.ic_prometheus = ICPrometheus(url="https://victoria.mainnet.dfinity.network/select/0/prometheus") self.ic_repo = ic_repo self.ignore_releases = ignore_releases or [] @@ -244,12 +217,7 @@ def reconcile(self): ) ) for rc_idx, rc in enumerate( - config.root.releases[ - : config.root.releases.index( - oldest_active_release(config, active_versions) - ) - + 1 - ] + config.root.releases[: config.root.releases.index(oldest_active_release(config, active_versions)) + 1] ): if rc.rc_name in self.ignore_releases: continue @@ -262,9 +230,7 @@ def reconcile(self): for v_idx, v in enumerate(rc.versions): logging.info("Updating version %s", v) push_release_tags(self.ic_repo, rc) - base_release_commit, base_release_name = find_base_release( - self.ic_repo, config, v.version - ) + base_release_commit, base_release_name = find_base_release(self.ic_repo, config, v.version) self.notes_client.ensure( base_release_commit=base_release_commit, base_release_tag=base_release_name, @@ -299,9 +265,7 @@ def reconcile(self): versions_to_unelect( config, active_versions=active_versions, - elected_versions=dre.get_blessed_versions()[ - "value" - ]["blessed_version_ids"], + elected_versions=dre.get_blessed_versions()["value"]["blessed_version_ids"], ), ) # This is a defensive approach in case the ic-admin exits with failure @@ -317,13 +281,9 @@ def reconcile(self): unelect_versions=unelect_versions, ) - versions_proposals = ( - self.governance_canister.replica_version_proposals() - ) + versions_proposals = self.governance_canister.replica_version_proposals() if v.version in versions_proposals: - self.state.save_proposal( - v.version, versions_proposals[v.version] - ) + self.state.save_proposal(v.version, versions_proposals[v.version]) # update the forum posts in case the proposal was created rc_forum_topic.update( @@ -373,9 +333,7 @@ def main(): else: load_dotenv() - watchdog = Watchdog( - timeout_seconds=600 - ) # Reconciler should report healthy every 10 minutes + watchdog = Watchdog(timeout_seconds=600) # Reconciler should report healthy every 10 minutes watchdog.start() discourse_client = DiscourseClient( @@ -384,9 +342,7 @@ def main(): api_key=os.environ["DISCOURSE_KEY"], ) config_loader = ( - GitReleaseLoader(f"https://github.com/{dre_repo}.git") - if "dev" not in os.environ - else DevReleaseLoader() + GitReleaseLoader(f"https://github.com/{dre_repo}.git") if "dev" not in os.environ else DevReleaseLoader() ) state = ReconcilerState( pathlib.Path( @@ -398,6 +354,9 @@ def main(): ) forum_client = ReleaseCandidateForumClient( discourse_client, + discourse_api_key=os.environ["DISCOURSE_KEY"], + discourse_url=os.environ["DISCOURSE_URL"], + discourse_username=os.environ["DISCOURSE_USER"], ) github_token = os.environ["GITHUB_TOKEN"] github_client = Github(auth=Auth.Token(github_token)) diff --git a/release-controller/test_forum.py b/release-controller/test_forum.py index 41a0200df..f788d6f28 100644 --- a/release-controller/test_forum.py +++ b/release-controller/test_forum.py @@ -23,7 +23,9 @@ def test_create_release_notes_on_new_release(): ) assert discourse_client.created_posts == [] assert discourse_client.created_topics == [] - forum_client = ReleaseCandidateForumClient(discourse_client=discourse_client) + forum_client = ReleaseCandidateForumClient( + discourse_client=discourse_client, discourse_api_key="", discourse_url="", discourse_username="" + ) post = forum_client.get_or_create( Release( rc_name="rc--2024-02-21_23-06", @@ -148,7 +150,9 @@ def test_create_post_in_new_category(): "title": "Proposal to elect new release rc--2024-02-21_23-06", } discourse_client.created_topics = [existing_topic] - forum_client = ReleaseCandidateForumClient(discourse_client=discourse_client) + forum_client = ReleaseCandidateForumClient( + discourse_client=discourse_client, discourse_api_key="", discourse_url="", discourse_username="" + ) post = forum_client.get_or_create( Release( rc_name="rc--2024-02-21_23-06", diff --git a/release-controller/test_reconciler.py b/release-controller/test_reconciler.py index d225e4927..0b4feb34e 100644 --- a/release-controller/test_reconciler.py +++ b/release-controller/test_reconciler.py @@ -13,7 +13,8 @@ from mock_google_docs import ReleaseNotesClientMock from publish_notes import PublishNotesClient from pydantic_yaml import parse_yaml_raw_as -from reconciler import find_base_release, oldest_active_release +from reconciler import find_base_release +from reconciler import oldest_active_release from reconciler import Reconciler from reconciler import ReconcilerState from reconciler import version_package_checksum @@ -38,7 +39,9 @@ def __del__(self): def test_e2e_mock_new_release(mocker): """Test the workflow when a new release is added to the index.""" discourse_client = DiscourseClientMock() - forum_client = ReleaseCandidateForumClient(discourse_client) + forum_client = ReleaseCandidateForumClient( + discourse_client, discourse_api_key="", discourse_url="", discourse_username="" + ) notes_client = ReleaseNotesClientMock() github_client = Github() mocker.patch.object(github_client, "get_repo") From 935c808ac681cafad71bbe52ddcf194d6ced2777 Mon Sep 17 00:00:00 2001 From: nikolamilosa Date: Wed, 9 Oct 2024 01:59:25 +0200 Subject: [PATCH 2/4] attempting to fix tests --- release-controller/test_forum.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/release-controller/test_forum.py b/release-controller/test_forum.py index f788d6f28..2b44820ea 100644 --- a/release-controller/test_forum.py +++ b/release-controller/test_forum.py @@ -24,7 +24,10 @@ def test_create_release_notes_on_new_release(): assert discourse_client.created_posts == [] assert discourse_client.created_topics == [] forum_client = ReleaseCandidateForumClient( - discourse_client=discourse_client, discourse_api_key="", discourse_url="", discourse_username="" + discourse_client=discourse_client, + discourse_api_key="", + discourse_url="https://forum.dfinity.org", + discourse_username="", ) post = forum_client.get_or_create( Release( @@ -151,7 +154,10 @@ def test_create_post_in_new_category(): } discourse_client.created_topics = [existing_topic] forum_client = ReleaseCandidateForumClient( - discourse_client=discourse_client, discourse_api_key="", discourse_url="", discourse_username="" + discourse_client=discourse_client, + discourse_api_key="", + discourse_url="https://forum.dfinity.org", + discourse_username="", ) post = forum_client.get_or_create( Release( From ec539ea5d60dd5def9be535a2551ea58ea64bb5b Mon Sep 17 00:00:00 2001 From: nikolamilosa Date: Wed, 9 Oct 2024 02:23:49 +0200 Subject: [PATCH 3/4] using different api for pagination --- release-controller/forum.py | 56 +++++++++++-------- .../mock_requests_post_fecher.py | 21 +++++++ release-controller/reconciler.py | 10 ++-- release-controller/test_forum.py | 5 +- 4 files changed, 62 insertions(+), 30 deletions(-) create mode 100644 release-controller/mock_requests_post_fecher.py diff --git a/release-controller/forum.py b/release-controller/forum.py index de318ef43..92b590271 100644 --- a/release-controller/forum.py +++ b/release-controller/forum.py @@ -27,6 +27,28 @@ def _post_template(changelog, version_name, proposal=None): """ +class RequestsPostFetcher: + """Posts fetcher using requests.""" + + def __init__( + self, + discourse_url: str, + discourse_username: str, + discourse_api_key: str, + ): + """Create a new post fetcher using requests.""" + self.discourse_url = discourse_url + self.discourse_username = discourse_username + self.discourse_api_key = discourse_api_key + + def fetch_topics_posts(self, topic_id): + """Return at maximum 1000 posts for a topic.""" + return requests.get( + f"{self.discourse_url}/t/{topic_id}.json?print=true", + headers={"Api-Username": self.discourse_username, "Api-Key": self.discourse_api_key}, + ).json() + + class ReleaseCandidateForumPost: """A post in a release candidate forum topic.""" @@ -45,17 +67,13 @@ def __init__( release: Release, client: DiscourseClient, nns_proposal_discussions_category, - discourse_url: str, - discourse_username: str, - discourse_api_key: str, + post_fetcher: RequestsPostFetcher, ): """Create a new topic.""" self.release = release self.client = client self.nns_proposal_discussions_category = nns_proposal_discussions_category - self.discourse_url = discourse_url - self.discourse_api_key = discourse_api_key - self.discourse_username = discourse_username + self.post_fetcher = post_fetcher topic = next( (t for t in client.topics_by(self.client.api_username) if self.release.rc_name in t["title"]), None, @@ -76,11 +94,7 @@ def __init__( def created_posts(self): """Return a list of posts created by the current user.""" - topic_posts = self.client.topic_posts(topic_id=self.topic_id) - topic_posts = requests.get( - f"{self.discourse_url}/t/{self.topic_id}.json?print=true", - headers={"Api-Username": self.discourse_username, "Api-Key": self.discourse_api_key}, - ).json() + topic_posts = self.post_fetcher.fetch_topics_posts(self.topic_id) if not topic_posts: raise RuntimeError("failed to list topic posts") @@ -155,9 +169,7 @@ def add_version(self, content: str): class ReleaseCandidateForumClient: """A client for interacting with release candidate forum topics.""" - def __init__( - self, discourse_client: DiscourseClient, discourse_url: str, discourse_username: str, discourse_api_key: str - ): + def __init__(self, discourse_client: DiscourseClient, post_fetcher: RequestsPostFetcher): """Create a new client.""" self.discourse_client = discourse_client self.nns_proposal_discussions_category = next( @@ -170,9 +182,7 @@ def __init__( "category" ], # hardcoded category id, seems like "include_subcategories" is not working ) - self.discourse_url = discourse_url - self.discourse_username = discourse_username - self.discourse_api_key = discourse_api_key + self.post_fetcher = post_fetcher def get_or_create(self, release: Release) -> ReleaseCandidateForumTopic: """Get or create a forum topic for the given release.""" @@ -180,9 +190,7 @@ def get_or_create(self, release: Release) -> ReleaseCandidateForumTopic: release=release, client=self.discourse_client, nns_proposal_discussions_category=self.nns_proposal_discussions_category, - discourse_url=self.discourse_url, - discourse_username=self.discourse_username, - discourse_api_key=self.discourse_api_key, + post_fetcher=self.post_fetcher, ) @@ -210,9 +218,11 @@ def main(): ) forum_client = ReleaseCandidateForumClient( discourse_client, - discourse_url=os.environ["DISCOURSE_URL"], - discourse_api_key=os.environ["DISCOURSE_KEY"], - discourse_username=os.environ["DISCOURSE_USER"], + RequestsPostFetcher( + discourse_api_key=os.environ["DISCOURSE_KEY"], + discourse_url=os.environ["DISCOURSE_URL"], + discourse_username=os.environ["DISCOURSE_USER"], + ), ) topic = forum_client.get_or_create(index.root.releases[0]) diff --git a/release-controller/mock_requests_post_fecher.py b/release-controller/mock_requests_post_fecher.py new file mode 100644 index 000000000..6b081f290 --- /dev/null +++ b/release-controller/mock_requests_post_fecher.py @@ -0,0 +1,21 @@ +from forum import RequestsPostFetcher + + +class MockRequestsPostFetcher(RequestsPostFetcher): + """A mock requests post fetcher client.""" + + def __init__(self, created_posts): + """Create mock post fetcher.""" + self.created_posts = created_posts + + def fetch_topics_posts(self, topic_id): + """Fetch mocked topics.""" + return { + "post_stream": { + "posts": [ + p + for p in [{"id": i + 1} | p for i, p in enumerate(self.created_posts)] + if p["topic_id"] == topic_id + ] + } + } diff --git a/release-controller/reconciler.py b/release-controller/reconciler.py index f6fa30e08..04daa27e9 100644 --- a/release-controller/reconciler.py +++ b/release-controller/reconciler.py @@ -18,7 +18,7 @@ import release_index import requests from dotenv import load_dotenv -from forum import ReleaseCandidateForumClient +from forum import ReleaseCandidateForumClient, RequestsPostFetcher from git_repo import GitRepo from git_repo import push_release_tags from github import Auth @@ -354,9 +354,11 @@ def main(): ) forum_client = ReleaseCandidateForumClient( discourse_client, - discourse_api_key=os.environ["DISCOURSE_KEY"], - discourse_url=os.environ["DISCOURSE_URL"], - discourse_username=os.environ["DISCOURSE_USER"], + RequestsPostFetcher( + discourse_api_key=os.environ["DISCOURSE_KEY"], + discourse_url=os.environ["DISCOURSE_URL"], + discourse_username=os.environ["DISCOURSE_USER"], + ), ) github_token = os.environ["GITHUB_TOKEN"] github_client = Github(auth=Auth.Token(github_token)) diff --git a/release-controller/test_forum.py b/release-controller/test_forum.py index 2b44820ea..8b50cfa28 100644 --- a/release-controller/test_forum.py +++ b/release-controller/test_forum.py @@ -1,6 +1,7 @@ import httpretty.utils from forum import ReleaseCandidateForumClient from mock_discourse import DiscourseClientMock +from mock_requests_post_fecher import MockRequestsPostFetcher from release_index import Release from release_index import Version @@ -155,9 +156,7 @@ def test_create_post_in_new_category(): discourse_client.created_topics = [existing_topic] forum_client = ReleaseCandidateForumClient( discourse_client=discourse_client, - discourse_api_key="", - discourse_url="https://forum.dfinity.org", - discourse_username="", + post_fetcher=MockRequestsPostFetcher(created_posts=discourse_client.created_posts), ) post = forum_client.get_or_create( Release( From bd0e5cce9f706e5fe8e4a157c01c95b50f18194b Mon Sep 17 00:00:00 2001 From: nikolamilosa Date: Wed, 9 Oct 2024 02:30:40 +0200 Subject: [PATCH 4/4] fixing tests --- release-controller/test_forum.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/release-controller/test_forum.py b/release-controller/test_forum.py index 8b50cfa28..a96d9cf4b 100644 --- a/release-controller/test_forum.py +++ b/release-controller/test_forum.py @@ -26,9 +26,7 @@ def test_create_release_notes_on_new_release(): assert discourse_client.created_topics == [] forum_client = ReleaseCandidateForumClient( discourse_client=discourse_client, - discourse_api_key="", - discourse_url="https://forum.dfinity.org", - discourse_username="", + post_fetcher=MockRequestsPostFetcher(created_posts=discourse_client.created_posts), ) post = forum_client.get_or_create( Release(