Create upgrade-cp-service.py #130
Conversation
WalkthroughA new script, Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as Command Line Interface
participant Parser as Argument Parser
participant Service as Upgrade Process (process_server)
participant Command as run_command
participant Pinger as wait_for_ping
participant Health as check_up
CLI->>Parser: Provide server arguments
Parser->>Service: Trigger upgrade process for each server
Service->>Command: Execute shell commands (e.g. mark backend as sick, upgrade packages)
Service->>Command: Initiate server reboot
Service->>Pinger: Ping server until it responds
Service->>Health: Check server API health
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
This is a brain dump and not supposed to work. I will make a real version before we next have to do it. |
| beta: Environment = { | ||
| 'wikidbname': 'metawikibeta', | ||
| 'wikiurl': 'meta.mirabeta.org', | ||
| 'servers': ['test151'], | ||
| } | ||
|
|
||
|
|
||
| prod: Environment = { | ||
| 'wikidbname': 'testwiki', | ||
| 'wikiurl': 'publictestwiki.com', | ||
| 'servers': [ | ||
| 'mw151', | ||
| 'mw152', | ||
| 'mw161', | ||
| 'mw162', | ||
| 'mw171', | ||
| 'mw172', | ||
| 'mw181', | ||
| 'mw182', | ||
| 'mwtask171', | ||
| 'mwtask181', | ||
| ], | ||
| } | ||
| ENVIRONMENTS: EnvironmentList = { | ||
| 'beta': beta, | ||
| 'prod': prod, | ||
| } | ||
| del beta | ||
| del prod |
There was a problem hiding this comment.
What if we didn't use intermediary variables for beta and prod?
| print(f"Waiting for {server} to come back online...") | ||
| start_time = time.time() | ||
| while time.time() - start_time < timeout: | ||
| response = os.system(f"ping -c 1 {server} > /dev/null 2>&1") |
| def run_command(command): | ||
| """Runs a shell command and returns the output.""" | ||
| try: | ||
| result = subprocess.run(command, shell=True, check=True, capture_output=True, text=True) |
| print(f"Timeout waiting for {server} to come back online.") | ||
| return False | ||
|
|
||
| def check_up(Debug: str, domain: str = 'meta.miraheze.org', verify: bool = True) -> bool: |
There was a problem hiding this comment.
Why is the variable called Debug and not debug
| return False | ||
|
|
||
| def check_up(Debug: str, domain: str = 'meta.miraheze.org', verify: bool = True) -> bool: | ||
| if verify is False: |
There was a problem hiding this comment.
False? But Debug is a string :p
| input('Press enter to continue') | ||
|
|
||
| if __name__ == "__main__": | ||
| parser = argparse.ArgumentParser(description='Process some integers.') |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
miraheze/puppet/upgrade-cp-service.py (2)
88-88: Remove or reuse the unusedlocationvariable.The variable is assigned but never utilised, triggering a warning. You can either remove it or integrate it into your logic.
- location = f'{domain}@{server}'🧰 Tools
🪛 Ruff (0.8.2)
88-88: Local variable
locationis assigned to but never usedRemove assignment to unused variable
location(F841)
🪛 GitHub Actions: Check Python
[error] 88-88: F841 local variable 'location' is assigned to but never used
1-142: Address coding style warnings.Several style checks fail (e.g. double quotes versus single quotes, insufficient blank lines). We recommend addressing them, for example by converting double quotes to single quotes and adding the required blank lines:
• Convert
"<text>"to'<text>'
• Ensure two blank lines before class and function definitions per PEP 8You can integrate a code formatter (e.g.
black,flake8) to automatically fix these issues.🧰 Tools
🪛 Ruff (0.8.2)
5-5: Undefined name
TypedDict(F821)
11-11: Undefined name
TypedDict(F821)
45-45: Undefined name
socket(F821)
47-47: Undefined name
argparse(F821)
50-50: Undefined name
get_environment_info(F821)
88-88: Local variable
locationis assigned to but never usedRemove assignment to unused variable
location(F841)
96-96: Undefined name
requests(F821)
96-96: Undefined name
port(F821)
137-137: Undefined name
argparse(F821)
140-140: Loop control variable
servernot used within loop bodyRename unused
serverto_server(B007)
140-140: Undefined name
servers(F821)
🪛 GitHub Actions: Check Python
[warning] 5-5: E302 expected 2 blank lines, found 1
[error] 5-5: F821 undefined name 'TypedDict'
[error] 11-11: F821 undefined name 'TypedDict'
[error] 45-45: F821 undefined name 'socket'
[warning] 47-47: E302 expected 2 blank lines, found 1
[error] 47-47: F821 undefined name 'argparse'
[error] 50-50: F821 undefined name 'get_environment_info'
[warning] 58-58: E302 expected 2 blank lines, found 1
[warning] 64-64: Q000 Double quotes found but single quotes preferred
[warning] 67-67: Q000 Double quotes found but single quotes preferred
[warning] 72-72: Q000 Double quotes found but single quotes preferred
[warning] 74-74: Q000 Double quotes found but single quotes preferred
[warning] 77-77: Q000 Double quotes found but single quotes preferred
[warning] 80-80: E302 expected 2 blank lines, found 1
[error] 88-88: F841 local variable 'location' is assigned to but never used
[error] 96-96: F821 undefined name 'requests'
[error] 96-96: F821 undefined name 'port'
[warning] 107-107: E302 expected 2 blank lines, found 1
[warning] 109-109: Q000 Double quotes found but single quotes preferred
[warning] 110-110: W293 blank line contains whitespace
[warning] 113-113: W293 blank line contains whitespace
[warning] 115-115: Q000 Double quotes found but single quotes preferred
[warning] 116-116: W293 blank line contains whitespace
[warning] 119-119: W293 blank line contains whitespace
[warning] 122-122: W293 blank line contains whitespace
[warning] 125-125: Q000 Double quotes found but single quotes preferred
[warning] 126-126: W293 blank line contains whitespace
[warning] 128-128: Q000 Double quotes found but single quotes preferred
[warning] 130-130: Q000 Double quotes found but single quotes preferred
[warning] 133-133: Q000 Double quotes found but single quotes preferred
[warning] 136-136: E305 expected 2 blank lines after class or function definition, found 1
[warning] 136-136: Q000 Double quotes found but single quotes preferred
[error] 137-137: F821 undefined name 'argparse'
[error] 140-140: F821 undefined name 'servers'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
miraheze/puppet/upgrade-cp-service.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
miraheze/puppet/upgrade-cp-service.py
5-5: Undefined name TypedDict
(F821)
11-11: Undefined name TypedDict
(F821)
45-45: Undefined name socket
(F821)
47-47: Undefined name argparse
(F821)
50-50: Undefined name get_environment_info
(F821)
88-88: Local variable location is assigned to but never used
Remove assignment to unused variable location
(F841)
96-96: Undefined name requests
(F821)
96-96: Undefined name port
(F821)
137-137: Undefined name argparse
(F821)
140-140: Loop control variable server not used within loop body
Rename unused server to _server
(B007)
140-140: Undefined name servers
(F821)
🪛 GitHub Actions: Check Python
miraheze/puppet/upgrade-cp-service.py
[warning] 5-5: E302 expected 2 blank lines, found 1
[error] 5-5: F821 undefined name 'TypedDict'
[error] 11-11: F821 undefined name 'TypedDict'
[error] 45-45: F821 undefined name 'socket'
[warning] 47-47: E302 expected 2 blank lines, found 1
[error] 47-47: F821 undefined name 'argparse'
[error] 50-50: F821 undefined name 'get_environment_info'
[warning] 58-58: E302 expected 2 blank lines, found 1
[warning] 64-64: Q000 Double quotes found but single quotes preferred
[warning] 67-67: Q000 Double quotes found but single quotes preferred
[warning] 72-72: Q000 Double quotes found but single quotes preferred
[warning] 74-74: Q000 Double quotes found but single quotes preferred
[warning] 77-77: Q000 Double quotes found but single quotes preferred
[warning] 80-80: E302 expected 2 blank lines, found 1
[error] 88-88: F841 local variable 'location' is assigned to but never used
[error] 96-96: F821 undefined name 'requests'
[error] 96-96: F821 undefined name 'port'
[warning] 107-107: E302 expected 2 blank lines, found 1
[warning] 109-109: Q000 Double quotes found but single quotes preferred
[warning] 110-110: W293 blank line contains whitespace
[warning] 113-113: W293 blank line contains whitespace
[warning] 115-115: Q000 Double quotes found but single quotes preferred
[warning] 116-116: W293 blank line contains whitespace
[warning] 119-119: W293 blank line contains whitespace
[warning] 122-122: W293 blank line contains whitespace
[warning] 125-125: Q000 Double quotes found but single quotes preferred
[warning] 126-126: W293 blank line contains whitespace
[warning] 128-128: Q000 Double quotes found but single quotes preferred
[warning] 130-130: Q000 Double quotes found but single quotes preferred
[warning] 133-133: Q000 Double quotes found but single quotes preferred
[warning] 136-136: E305 expected 2 blank lines after class or function definition, found 1
[warning] 136-136: Q000 Double quotes found but single quotes preferred
[error] 137-137: F821 undefined name 'argparse'
[error] 140-140: F821 undefined name 'servers'
| import subprocess | ||
| import time | ||
| import os | ||
|
|
There was a problem hiding this comment.
Add missing imports to fix NameErrors.
The script makes use of TypedDict, requests, argparse, and socket but fails to import them, causing NameErrors. Please add these imports:
+import argparse
+import requests
+import socket
+from typing import TypedDict📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import subprocess | |
| import time | |
| import os | |
| import argparse | |
| import requests | |
| import socket | |
| from typing import TypedDict | |
| import subprocess | |
| import time | |
| import os |
| if debug_access_key: | ||
| headers['X-WikiTide-Debug-Access-Key'] = debug_access_key | ||
| up = False | ||
| req = requests.get(f'https://{domain}:{port}/w/api.php?action=query&meta=siteinfo&formatversion=2&format=json', headers=headers, verify=verify) |
There was a problem hiding this comment.
Provide a default value for port and ensure requests is imported.
The code references requests and port, but requests is not imported (addressed in a prior comment) and port is undefined. You can fix this by supplying a default parameter and importing requests:
-def check_up(Debug: str, domain: str = 'meta.miraheze.org', verify: bool = True) -> bool:
+def check_up(Debug: str, domain: str = 'meta.miraheze.org', verify: bool = True, port: int = 443) -> bool:
...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| req = requests.get(f'https://{domain}:{port}/w/api.php?action=query&meta=siteinfo&formatversion=2&format=json', headers=headers, verify=verify) | |
| def check_up(Debug: str, domain: str = 'meta.miraheze.org', verify: bool = True, port: int = 443) -> bool: | |
| # ... (other code in the function) | |
| req = requests.get( | |
| f'https://{domain}:{port}/w/api.php?action=query&meta=siteinfo&formatversion=2&format=json', | |
| headers=headers, | |
| verify=verify | |
| ) | |
| # ... (rest of the function) |
🧰 Tools
🪛 Ruff (0.8.2)
96-96: Undefined name requests
(F821)
96-96: Undefined name port
(F821)
🪛 GitHub Actions: Check Python
[error] 96-96: F821 undefined name 'requests'
[error] 96-96: F821 undefined name 'port'
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Docstrings generation was requested by @RhinosF1. * #130 (comment) The following files were modified: * `miraheze/puppet/upgrade-cp-service.py`
|
Note Generated docstrings for this pull request at #131 |
Co-authored-by: Claire <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Docstrings generation was requested by @RhinosF1. * #130 (comment) The following files were modified: * `miraheze/puppet/upgrade-cp-service.py` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: RhinosF1 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
miraheze/puppet/upgrade-cp-service.py (6)
57-57:⚠️ Potential issueDefine the get_environment_info function.
The function
get_environment_info()is called but not defined anywhere in the code.Add a function definition for
get_environment_info():def get_environment_info(): """ Get the environment information based on the current hostname. Returns: Environment: The environment configuration for the current host. """ if HOSTNAME in ENVIRONMENTS['beta']['servers']: return ENVIRONMENTS['beta'] else: return ENVIRONMENTS['prod']🧰 Tools
🪛 Ruff (0.8.2)
57-57: Undefined name
get_environment_info(F821)
🪛 GitHub Actions: Check Python
[error] 57-57: F821 undefined name 'get_environment_info'
81-81: 🛠️ Refactor suggestionAvoid using shell=True for security reasons.
Using
shell=Truecan be a security risk if the command contains user-supplied input.- result = subprocess.run(command, shell=True, check=True, capture_output=True, text=True) + result = subprocess.run(command.split(), check=True, capture_output=True, text=True)For complex commands that need shell features, consider:
result = subprocess.run(['bash', '-c', command], check=True, capture_output=True, text=True)
106-106: 🛠️ Refactor suggestionUse run_command() instead of os.system().
The script already has a
run_command()function for executing shell commands, which handles errors and returns output.- response = os.system(f"ping -c 1 {server} > /dev/null 2>&1") - if response == 0: + response = run_command(f"ping -c 1 {server}") + if response is not None:🧰 Tools
🪛 GitHub Actions: Check Python
[error] 106-106: Q000 Double quotes found but single quotes preferred
149-149:⚠️ Potential issueAdd port parameter with default value.
The function uses the
portvariable but it's not defined anywhere.-def check_up(Debug: str, domain: str = 'meta.miraheze.org', verify: bool = True) -> bool: +def check_up(Debug: str, domain: str = 'meta.miraheze.org', verify: bool = True, port: int = 443) -> bool:🧰 Tools
🪛 Ruff (0.8.2)
149-149: Undefined name
requests(F821)
149-149: Undefined name
port(F821)
🪛 GitHub Actions: Check Python
[error] 149-149: F821 undefined name 'requests'
[error] 149-149: F821 undefined name 'port'
204-205:⚠️ Potential issueFix the server processing loop.
The loop uses the correct loop variable
serverbut then passes the entire server list to theprocess_serverfunction.for server in args.servers: - process_server(args.servers) + process_server(server)🧰 Tools
🪛 Ruff (0.8.2)
204-204: Loop control variable
servernot used within loop bodyRename unused
serverto_server(B007)
1-4:⚠️ Potential issueAdd missing imports.
Several imports are missing which will cause NameErrors. You need to import TypedDict, argparse, socket, and requests.
import subprocess import time import os +import socket +import argparse +import requests +from typing import TypedDict
🧹 Nitpick comments (2)
miraheze/puppet/upgrade-cp-service.py (2)
5-9: Follow proper spacing conventions.Python style guides recommend two blank lines before class definitions.
import subprocess import time import os +import socket +import argparse +import requests +from typing import TypedDict + class Environment(TypedDict): wikidbname: str wikiurl: str servers: list🧰 Tools
🪛 Ruff (0.8.2)
5-5: Undefined name
TypedDict(F821)
🪛 GitHub Actions: Check Python
[error] 5-5: E302 expected 2 blank lines, found 1
[error] 5-5: F821 undefined name 'TypedDict'
141-141: Remove unused variable.The variable
locationis assigned but never used in the code.- location = f'{domain}@{server}'🧰 Tools
🪛 Ruff (0.8.2)
141-141: Local variable
locationis assigned to but never usedRemove assignment to unused variable
location(F841)
🪛 GitHub Actions: Check Python
[error] 141-141: F841 local variable 'location' is assigned to but never used
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
miraheze/puppet/upgrade-cp-service.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
miraheze/puppet/upgrade-cp-service.py
5-5: Undefined name TypedDict
(F821)
11-11: Undefined name TypedDict
(F821)
45-45: Undefined name socket
(F821)
47-47: Undefined name argparse
(F821)
57-57: Undefined name get_environment_info
(F821)
141-141: Local variable location is assigned to but never used
Remove assignment to unused variable location
(F841)
149-149: Undefined name requests
(F821)
149-149: Undefined name port
(F821)
201-201: Undefined name argparse
(F821)
204-204: Loop control variable server not used within loop body
Rename unused server to _server
(B007)
🪛 GitHub Actions: Check Python
miraheze/puppet/upgrade-cp-service.py
[error] 5-5: E302 expected 2 blank lines, found 1
[error] 5-5: F821 undefined name 'TypedDict'
[error] 11-11: F821 undefined name 'TypedDict'
[error] 45-45: F821 undefined name 'socket'
[error] 47-47: E302 expected 2 blank lines, found 1
[error] 47-47: F821 undefined name 'argparse'
[error] 51-51: W293 blank line contains whitespace
[error] 57-57: F821 undefined name 'get_environment_info'
[error] 65-65: E302 expected 2 blank lines, found 1
[error] 68-68: W293 blank line contains whitespace
[error] 73-73: W293 blank line contains whitespace
[error] 76-76: W293 blank line contains whitespace
[error] 84-84: Q000 Double quotes found but single quotes preferred
[error] 87-87: E302 expected 2 blank lines, found 1
[error] 90-90: W293 blank line contains whitespace
[error] 94-94: W293 blank line contains whitespace
[error] 99-99: W293 blank line contains whitespace
[error] 103-103: Q000 Double quotes found but single quotes preferred
[error] 106-106: Q000 Double quotes found but single quotes preferred
[error] 108-108: Q000 Double quotes found but single quotes preferred
[error] 111-111: Q000 Double quotes found but single quotes preferred
[error] 114-114: E302 expected 2 blank lines, found 1
[error] 117-117: W293 blank line contains whitespace
[error] 125-125: W293 blank line contains whitespace
[error] 130-130: W293 blank line contains whitespace
[error] 141-141: F841 local variable 'location' is assigned to but never used
[error] 149-149: F821 undefined name 'requests'
[error] 149-149: F821 undefined name 'port'
[error] 160-160: E302 expected 2 blank lines, found 1
[error] 163-163: W293 blank line contains whitespace
[error] 169-169: W293 blank line contains whitespace
[error] 173-173: Q000 Double quotes found but single quotes preferred
[error] 174-174: W293 blank line contains whitespace
[error] 176-176: Q000 Double quotes found but single quotes preferred
[error] 177-177: W293 blank line contains whitespace
[error] 179-179: Q000 Double quotes found but single quotes preferred
[error] 180-180: W293 blank line contains whitespace
[error] 183-183: W293 blank line contains whitespace
[error] 186-186: W293 blank line contains whitespace
[error] 189-189: Q000 Double quotes found but single quotes preferred
[error] 190-190: W293 blank line contains whitespace
[error] 192-192: Q000 Double quotes found but single quotes preferred
[error] 194-194: Q000 Double quotes found but single quotes preferred
[error] 197-197: Q000 Double quotes found but single quotes preferred
[error] 200-200: E305 expected 2 blank lines after class or function definition, found 1
[error] 200-200: Q000 Double quotes found but single quotes preferred
[error] 201-201: F821 undefined name 'argparse'
🔇 Additional comments (5)
miraheze/puppet/upgrade-cp-service.py (5)
16-44: Consider simplifying environment configuration.The current approach with intermediate variables for beta and prod is more readable, but it can be simplified by directly assigning to ENVIRONMENTS.
-beta: Environment = { - 'wikidbname': 'metawikibeta', - 'wikiurl': 'meta.mirabeta.org', - 'servers': ['test151'], -} - - -prod: Environment = { - 'wikidbname': 'testwiki', - 'wikiurl': 'publictestwiki.com', - 'servers': [ - 'mw151', - 'mw152', - 'mw161', - 'mw162', - 'mw171', - 'mw172', - 'mw181', - 'mw182', - 'mwtask171', - 'mwtask181', - ], -} ENVIRONMENTS: EnvironmentList = { - 'beta': beta, - 'prod': prod, + 'beta': { + 'wikidbname': 'metawikibeta', + 'wikiurl': 'meta.mirabeta.org', + 'servers': ['test151'], + }, + 'prod': { + 'wikidbname': 'testwiki', + 'wikiurl': 'publictestwiki.com', + 'servers': [ + 'mw151', + 'mw152', + 'mw161', + 'mw162', + 'mw171', + 'mw172', + 'mw181', + 'mw182', + 'mwtask171', + 'mwtask181', + ], + }, } -del beta -del prod
45-45: Simplify hostname extraction.The current approach works but can be simplified.
-HOSTNAME = socket.gethostname().partition('.')[0] +HOSTNAME = socket.gethostname().split('.')[0]🧰 Tools
🪛 Ruff (0.8.2)
45-45: Undefined name
socket(F821)
🪛 GitHub Actions: Check Python
[error] 45-45: F821 undefined name 'socket'
114-114: Use lowercase for variable names.Variable names should use lowercase letters with words separated by underscores (snake_case).
-def check_up(Debug: str, domain: str = 'meta.miraheze.org', verify: bool = True) -> bool: +def check_up(debug: str, domain: str = 'meta.miraheze.org', verify: bool = True, port: int = 443) -> bool:🧰 Tools
🪛 GitHub Actions: Check Python
[error] 114-114: E302 expected 2 blank lines, found 1
134-134: Simplify boolean comparison.Comparing a boolean value with
is Falseis less Pythonic than usingnot.- if verify is False: + if not verify:
201-201: Update the argument parser description.The current description "Process some integers" doesn't match the script's purpose.
- parser = argparse.ArgumentParser(description='Process some integers.') + parser = argparse.ArgumentParser(description='Upgrade CP services by processing servers sequentially.')🧰 Tools
🪛 Ruff (0.8.2)
201-201: Undefined name
argparse(F821)
🪛 GitHub Actions: Check Python
[error] 201-201: F821 undefined name 'argparse'
Summary by CodeRabbit