Skip to content

fix_container_permissions: rewrite to use mwscript#205

Open
Universal-Omega wants to merge 3 commits intomainfrom
fix_container_permissions-mwscript
Open

fix_container_permissions: rewrite to use mwscript#205
Universal-Omega wants to merge 3 commits intomainfrom
fix_container_permissions-mwscript

Conversation

@Universal-Omega
Copy link
Member

@Universal-Omega Universal-Omega commented Jan 18, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Improved detection and handling of container permission errors with more reliable verification and a final check to ensure changes complete successfully.
  • Refactor

    • Streamlined container access workflow and output pattern processing for more consistent and robust operations.
  • New Features

    • Added a command-line entry point to run the container-permissions tool directly.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 18, 2026

Walkthrough

Adds a helper run_mwscript_setcontainersaccess and refactors fix_container_perms to use it, parse combined stdout/stderr for container names, run per-container swift post ACL updates, and perform a final verification call. Includes a CLI entry point via argparse.

Changes

Cohort / File(s) Summary
Container permissions refactor
miraheze/swift/fix_container_permissions.py
Added run_mwscript_setcontainersaccess(wiki: str, check: bool) helper; refactored fix_container_perms to call the helper for initial and final checks, parse combined stdout/stderr (regex) to extract container names, iterate matched containers to run swift post with updated ACLs, and added argparse CLI entry.

Sequence Diagram(s)

sequenceDiagram
    participant Script as Script (fix_container_perms)
    participant MWScript as mwscript CreateWiki:SetContainersAccess
    participant Parser as Output Parser
    participant Swift as swift (post)
    Script->>MWScript: run_mwscript_setcontainersaccess(wiki, check=False)
    MWScript-->>Script: combined stdout/stderr
    Script->>Parser: parse output for container names (regex)
    Parser-->>Script: list of containers
    loop for each container
        Script->>Swift: swift post <container> --read-acl ...
        Swift-->>Script: success/failure
    end
    Script->>MWScript: run_mwscript_setcontainersaccess(wiki, check=True)
    MWScript-->>Script: final verification result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped through output, sniffed each name,
parsed the list and set their claim,
swift doors opened, ACLs in tune,
a tidy fix beneath the moon. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately describes the primary change: rewriting fix_container_permissions to use mwscript instead of previous implementation methods.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@miraheze/swift/fix_container_permissions.py`:
- Around line 6-12: The helper currently invokes the bare command name
('mwscript') which is vulnerable to PATH hijacking and may break imports if
executables are missing; update run_mwscript_setcontainersaccess to resolve and
use an absolute executable path (e.g., via shutil.which at call time or from a
module-level configurable constant) and raise a clear, non-import-time error if
resolution fails; ensure the resolved path is passed to subprocess.run instead
of the bare name and apply the same resolution pattern to any other functions
that call 'swift' or similar executables in this module to avoid module-level
side effects.
- Around line 15-20: The function fix_container_perms currently calls
run_mwscript_setcontainersaccess and then silently proceeds if the subprocess
failed but produced no matches; update fix_container_perms to check the
return/exit status from run_mwscript_setcontainersaccess (or inspect
out.returncode) and if it is non-zero and the regex matches list (variable
matches) is empty, raise an exception or log and abort so the failure is not
ignored; keep using run_mwscript_setcontainersaccess and the existing regex but
add this explicit fail-fast branch before continuing to process any matches.
🧹 Nitpick comments (1)
miraheze/swift/fix_container_permissions.py (1)

37-42: Validate --wiki format before use.
A simple pattern check avoids invalid container names and clearer error messages.

🔧 Proposed fix
 def main() -> None:
     parser = argparse.ArgumentParser(description='Fix container permissions for a specified wiki')
     parser.add_argument('--wiki', required=True, help='wiki database name')
 
     args = parser.parse_args()
+    if not re.fullmatch(r'[a-z0-9_]+', args.wiki):
+        parser.error('Invalid wiki database name format')
     fix_container_perms(args.wiki)

Comment on lines +6 to +12
def run_mwscript_setcontainersaccess(wiki: str, check: bool) -> subprocess.CompletedProcess[str]:
return subprocess.run(
['mwscript', 'CreateWiki:SetContainersAccess', wiki, '--yes'],
capture_output=True,
text=True,
check=check,
)
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n miraheze/swift/fix_container_permissions.py

Repository: miraheze/python-functions

Length of output: 1642


🏁 Script executed:

rg -t py "subprocess.run\|subprocess.Popen\|os.system" --context 2 | head -100

Repository: miraheze/python-functions

Length of output: 51


🏁 Script executed:

fd -t f -e py | head -20 && echo "---" && rg "subprocess" --no-heading | head -40

Repository: miraheze/python-functions

Length of output: 1845


🏁 Script executed:

cat -n miraheze/mediawiki/mwscript.py | head -50

Repository: miraheze/python-functions

Length of output: 1447


🏁 Script executed:

cat -n miraheze/mediawiki/mwscript.py | sed -n '120,150p'

Repository: miraheze/python-functions

Length of output: 1584


🏁 Script executed:

cat -n miraheze/mediawiki/mwimport.py | head -70

Repository: miraheze/python-functions

Length of output: 3758


🏁 Script executed:

cat -n miraheze/mediawiki/mwimport.py | grep -A 8 "subprocess.run\|subprocess.Popen"

Repository: miraheze/python-functions

Length of output: 864


🏁 Script executed:

rg "mwscript|swift" --type py -B 2 -A 2

Repository: miraheze/python-functions

Length of output: 16389


🏁 Script executed:

rg "swift|mwscript" --type py -B 1 -A 1 | grep -i "bin\|path" | head -20

Repository: miraheze/python-functions

Length of output: 1314


🏁 Script executed:

# Check if there are any references to absolute paths for mwscript or swift in the repo
rg "mwscript|swift" --type py --type sh --type json | grep "/" | head -20

Repository: miraheze/python-functions

Length of output: 2849


🏁 Script executed:

# Search for any documentation or setup that mentions mwscript or swift paths
find . -type f \( -name "*.md" -o -name "*.rst" -o -name "setup.py" -o -name "setup.cfg" -o -name "pyproject.toml" \) -exec grep -l "mwscript\|swift" {} \;

Repository: miraheze/python-functions

Length of output: 80


🏁 Script executed:

cat -n setup.cfg

Repository: miraheze/python-functions

Length of output: 1341


🏁 Script executed:

# Check README and other documentation
ls -la | grep -i readme && cat README.md | head -100

Repository: miraheze/python-functions

Length of output: 147


Use absolute paths or document PATH requirements for mwscript and swift.
Bare command names are vulnerable to PATH hijacking. Either resolve to absolute paths (preferred per codebase pattern) or document that these commands must be available in PATH. Avoid module-level initialisation that breaks imports if executables are unavailable.

🧰 Tools
🪛 Ruff (0.14.13)

7-7: subprocess call: check for execution of untrusted input

(S603)


8-8: Starting a process with a partial executable path

(S607)

🤖 Prompt for AI Agents
In `@miraheze/swift/fix_container_permissions.py` around lines 6 - 12, The helper
currently invokes the bare command name ('mwscript') which is vulnerable to PATH
hijacking and may break imports if executables are missing; update
run_mwscript_setcontainersaccess to resolve and use an absolute executable path
(e.g., via shutil.which at call time or from a module-level configurable
constant) and raise a clear, non-import-time error if resolution fails; ensure
the resolved path is passed to subprocess.run instead of the bare name and apply
the same resolution pattern to any other functions that call 'swift' or similar
executables in this module to avoid module-level side effects.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mwscript is a function available within the package, you could just call it as a function. That would probably be cleaner.

We can assume that Swift is on the PATH when running a Swift maintenance script so I think that element is safe in this context.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Examine the mwscript function interface
echo "=== mwscript.py function interface ==="
cat miraheze/mediawiki/mwscript.py

echo -e "\n=== Looking for examples of mwscript function calls ==="
rg "from miraheze.mediawiki.mwscript import|from miraheze.mediawiki import mwscript|import miraheze.mediawiki.mwscript" -A 5

Length of output: 6175


@RhinosF1, you're absolutely right about using the mwscript function from the package - that's a much cleaner approach!

Looking at the mwscript.py code, I can see the challenge: the current implementation needs to capture stdout/stderr to parse for failed containers, but mwscript.run() uses os.system() which doesn't support output capture.

Here's a suggested approach that leverages the mwscript module while still capturing output:

from miraheze.mediawiki import mwscript
import subprocess
import re

def run_mwscript_setcontainersaccess(wiki: str, check: bool) -> subprocess.CompletedProcess[str]:
    # Build the command using mwscript's logic
    args = mwscript.get_args(['CreateWiki:SetContainersAccess', wiki, '--yes'])
    commands = mwscript.syscheck(mwscript.get_commands(args))
    
    # Run the command with output capture
    return subprocess.run(
        commands['command'],
        shell=True,
        capture_output=True,
        text=True,
        check=check,
    )

This approach:

  • Uses mwscript.get_commands() to build the proper command (handling versioning, sudo, etc.)
  • Still captures output for parsing failed containers
  • Avoids PATH hijacking by using the package function

Alternatively, if you'd prefer to enhance mwscript.run() to support output capture mode, that would benefit other use cases too.

Regarding Swift - acknowledged that it's safe to assume on PATH in this maintenance script context. ✓


🧬 Code Graph Analysis Results

[miraheze/swift/fix_container_permissions.py:6-12]

def run_mwscript_setcontainersaccess(wiki: str, check: bool) -> subprocess.CompletedProcess[str]:
return subprocess.run(
['mwscript', 'CreateWiki:SetContainersAccess', wiki, '--yes'],
capture_output=True,
text=True,
check=check,
)

[miraheze/swift/fix_container_permissions.py:15-34]

def fix_container_perms(wiki: str) -> None:
out = run_mwscript_setcontainersaccess(wiki, check=False)
matches = re.findall(
r"Making sure 'mwstore://miraheze-swift/([^']+)' [^\n]+.failed.",
(out.stdout or '') + '\n' + (out.stderr or ''),
)

for container in matches:
    subprocess.run(
        [
            'swift',
            'post',
            '--read-acl', 'mw:media',
            '--write-acl', 'mw:media',
            f'miraheze-{wiki}-{container}',
        ],
        check=True,
    )

run_mwscript_setcontainersaccess(wiki, check=True)

[miraheze/swift/fix_container_permissions.py:37-46]

def main() -> None:
parser = argparse.ArgumentParser(description='Fix container permissions for a specified wiki')
parser.add_argument('--wiki', required=True, help='wiki database name')

args = parser.parse_args()
fix_container_perms(args.wiki)

[miraheze/swift/fix_container_permissions.py:45-45]

if name == 'main':
main()


✏️ Learnings added
Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 205
File: miraheze/swift/fix_container_permissions.py:6-12
Timestamp: 2026-01-19T07:31:09.535Z
Learning: The `mwscript` command is available as a callable function within the `miraheze.mediawiki.mwscript` module. Scripts should import and call `mwscript.get_args()`, `mwscript.get_commands()`, and related functions rather than invoking 'mwscript' as an external command via subprocess.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 205
File: miraheze/swift/fix_container_permissions.py:6-12
Timestamp: 2026-01-19T07:31:09.535Z
Learning: For Swift maintenance scripts in the miraheze/python-functions repository, it's safe to assume that the `swift` command is available on PATH, as these scripts are specifically designed to run in environments where Swift is installed.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 2
File: miraheze/swift/fix_container_permissions.py:0-0
Timestamp: 2024-10-08T17:05:15.088Z
Learning: The `fix_container_permissions.py` script was updated to replace `os.system` calls with `subprocess.run` for enhanced security, and the unused `os` import was subsequently removed.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/mediawiki/mwscript.py:128-147
Timestamp: 2024-07-26T21:00:05.787Z
Learning: The `run` function in `miraheze/mediawiki/mwscript.py` can be enhanced by adding detailed error handling and retry mechanisms for executing external commands.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/mediawiki/mwscript.py:128-147
Timestamp: 2024-10-08T17:05:15.088Z
Learning: The `run` function in `miraheze/mediawiki/mwscript.py` can be enhanced by adding detailed error handling and retry mechanisms for executing external commands.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/puppet/disable_puppet.py:8-10
Timestamp: 2024-10-08T17:05:15.088Z
Learning: In the `disable_puppet.py` script, use `argparse` for argument parsing and `subprocess.run` for system calls to improve security and error handling.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/puppet/disable_puppet.py:8-10
Timestamp: 2024-07-26T21:00:05.787Z
Learning: In the `disable_puppet.py` script, use `argparse` for argument parsing and `subprocess.run` for system calls to improve security and error handling.

Learnt from: BlankEclair
Repo: miraheze/python-functions PR: 119
File: miraheze/mediawiki/mwimport.py:111-143
Timestamp: 2025-03-07T10:07:54.770Z
Learning: When using subprocess with argument lists in Python (not using shell=True), command injection concerns are minimized as arguments are passed directly to the executable without shell interpretation. In the mwimport.py script, user input within f-strings in the middle of command arguments doesn't present a significant security risk in this context.

Learnt from: BlankEclair
Repo: miraheze/python-functions PR: 119
File: miraheze/mediawiki/mwimport.py:111-146
Timestamp: 2025-03-07T10:05:19.313Z
Learning: In Miraheze's Python scripts, subprocess calls that take a list of arguments (rather than a shell string) are considered safe from shell injection as they don't pass through a shell interpreter.

Learnt from: BlankEclair
Repo: miraheze/python-functions PR: 119
File: miraheze/mediawiki/mwimport.py:52-57
Timestamp: 2025-03-07T10:00:33.640Z
Learning: The `miraheze/mediawiki/mwimport.py` script is specifically designed to run on Miraheze's MediaWiki servers (mw* servers), where system dependencies like `/usr/local/bin/logsalmsg` are expected to be present. Portability to other environments is not a concern for this tool.

Learnt from: BlankEclair
Repo: miraheze/python-functions PR: 119
File: miraheze/mediawiki/mwimport.py:59-66
Timestamp: 2025-03-07T09:54:27.055Z
Learning: The MediaWiki import tools in miraheze/python-functions are specifically designed to run only on mw* servers, where system dependencies like `/usr/local/bin/getMWVersion` and `/usr/local/bin/logsalmsg` are available.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/mediawiki/check_read_only.py:6-6
Timestamp: 2024-10-08T17:05:15.088Z
Learning: RhinosF1 prefers to keep `verify=False` for requests to localhost, considering it safe.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/mediawiki/check_read_only.py:6-6
Timestamp: 2024-07-26T21:00:05.787Z
Learning: RhinosF1 prefers to keep `verify=False` for requests to localhost, considering it safe.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updating mwscript to use subprocess.run() is acceptable to me

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RhinosF1, excellent! Updating mwscript to use subprocess.run() will make it much more flexible and allow for proper output capture across all scripts that need it.

Would you like me to help plan the implementation? I can:

  1. Update mwscript.run() to use subprocess.run() instead of os.system(), adding a parameter to control whether output should be captured
  2. Update fix_container_permissions.py to import and use the enhanced mwscript function directly
  3. Ensure backwards compatibility for existing scripts that currently use mwscript.run()

This refactor would benefit not just this script, but any future scripts that need to capture MediaWiki maintenance script output.

Shall I proceed with planning these changes?


🧠 Learnings used
Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 2
File: miraheze/swift/fix_container_permissions.py:0-0
Timestamp: 2024-10-08T17:05:15.088Z
Learning: The `fix_container_permissions.py` script was updated to replace `os.system` calls with `subprocess.run` for enhanced security, and the unused `os` import was subsequently removed.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/mediawiki/mwscript.py:128-147
Timestamp: 2024-07-26T21:00:05.787Z
Learning: The `run` function in `miraheze/mediawiki/mwscript.py` can be enhanced by adding detailed error handling and retry mechanisms for executing external commands.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/mediawiki/mwscript.py:128-147
Timestamp: 2024-10-08T17:05:15.088Z
Learning: The `run` function in `miraheze/mediawiki/mwscript.py` can be enhanced by adding detailed error handling and retry mechanisms for executing external commands.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/puppet/disable_puppet.py:8-10
Timestamp: 2024-10-08T17:05:15.088Z
Learning: In the `disable_puppet.py` script, use `argparse` for argument parsing and `subprocess.run` for system calls to improve security and error handling.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/puppet/disable_puppet.py:8-10
Timestamp: 2024-07-26T21:00:05.787Z
Learning: In the `disable_puppet.py` script, use `argparse` for argument parsing and `subprocess.run` for system calls to improve security and error handling.

Learnt from: BlankEclair
Repo: miraheze/python-functions PR: 119
File: miraheze/mediawiki/mwimport.py:111-143
Timestamp: 2025-03-07T10:07:54.770Z
Learning: When using subprocess with argument lists in Python (not using shell=True), command injection concerns are minimized as arguments are passed directly to the executable without shell interpretation. In the mwimport.py script, user input within f-strings in the middle of command arguments doesn't present a significant security risk in this context.

Learnt from: BlankEclair
Repo: miraheze/python-functions PR: 119
File: miraheze/mediawiki/mwimport.py:111-146
Timestamp: 2025-03-07T10:05:19.313Z
Learning: In Miraheze's Python scripts, subprocess calls that take a list of arguments (rather than a shell string) are considered safe from shell injection as they don't pass through a shell interpreter.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 205
File: miraheze/swift/fix_container_permissions.py:6-12
Timestamp: 2026-01-19T07:31:09.535Z
Learning: For Swift maintenance scripts in the miraheze/python-functions repository, it's safe to assume that the `swift` command is available on PATH, as these scripts are specifically designed to run in environments where Swift is installed.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 205
File: miraheze/swift/fix_container_permissions.py:6-12
Timestamp: 2026-01-19T07:31:09.535Z
Learning: The `mwscript` command is available as a callable function within the `miraheze.mediawiki.mwscript` module. Scripts should import and call `mwscript.get_args()`, `mwscript.get_commands()`, and related functions rather than invoking 'mwscript' as an external command via subprocess.

Learnt from: BlankEclair
Repo: miraheze/python-functions PR: 119
File: miraheze/mediawiki/mwimport.py:111-146
Timestamp: 2025-03-07T10:05:19.313Z
Learning: Miraheze's Python scripts use a simple "Type 'Y' to confirm" prompt pattern for user confirmation, consistent with existing implementations like in miraheze/mediawiki/mwscript.py.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/mediawiki/check_read_only.py:6-6
Timestamp: 2024-10-08T17:05:15.088Z
Learning: RhinosF1 prefers to keep `verify=False` for requests to localhost, considering it safe.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/mediawiki/check_read_only.py:6-6
Timestamp: 2024-07-26T21:00:05.787Z
Learning: RhinosF1 prefers to keep `verify=False` for requests to localhost, considering it safe.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

@RhinosF1, great! Let me examine the current implementation and existing usages to ensure we maintain backwards compatibility while adding the new functionality.


🏁 Script executed:

#!/bin/bash
# Examine the full mwscript.py implementation
echo "=== Full mwscript.py content ==="
cat miraheze/mediawiki/mwscript.py

echo -e "\n=== All usages of mwscript.run() in the codebase ==="
rg "mwscript\.run\(" -n -C3

echo -e "\n=== All imports of mwscript module ==="
rg "from miraheze\.mediawiki import mwscript|import miraheze\.mediawiki\.mwscript" -n

Length of output: 6014


@RhinosF1, perfect! Based on my analysis, I can see that mwscript.run() currently uses os.system() and there are no existing usages in the codebase (except tests), which makes this refactor quite safe. Let me plan the implementation:

Task: Refactor mwscript to use subprocess.run() and update fix_container_permissions.py

Changes overview:

  1. Update mwscript.run() to use subprocess.run() with optional output capture
  2. Update fix_container_permissions.py to import and use mwscript module functions
  3. Maintain backwards compatibility for existing behavior

Benefits:

  • ✅ Enhanced security (subprocess.run vs os.system)
  • ✅ Output capture capability for scripts that need it
  • ✅ Better error handling with returncode access
  • ✅ Consistent pattern across all MediaWiki scripts

This refactor will make the code cleaner, more maintainable, and provide the output capture functionality needed for fix_container_permissions.py while preserving all existing behavior for other scripts that might use mwscript in the future.


🧬 Code Graph Analysis Results
  • File: miraheze/mediawiki/mwimport.py (lines 15-65)
def parse_args(input_args: list | None = None, check_paths: bool = True) -> argparse.Namespace:
    parser = argparse.ArgumentParser(description='A script to automate manual wiki imports')
    parser.add_argument(
        '--no-log', dest='nolog', action='store_true',
        help='Whether or not to disable logging to the server admin log',
    )
    parser.add_argument(
        '--confirm', '--yes', '-y', dest='confirm', action='store_true',
        help='Whether or not to skip the initial confirmation prompt',
    )
    parser.add_argument('--version', help='MediaWiki version to use (automatically detected if not passed)')
    parser.add_argument('--xml', help='XML file to import')
    parser.add_argument('--username-prefix', help='Interwiki prefix for importing XML')
    parser.add_argument('--images', help='Directory of images to import')
    parser.add_argument(
        '--images-comment', help='The comment passed to importImages.php'
        ' (example: "Importing images from https://example.com ([[phorge:T1234|T1234]])")',
    )
    parser.add_argument(
        '--search-recursively', action='store_true',
        help='Whether or not to pass --search-recursively (check files in subdirectories) to importImages.php',
    )
    parser.add_argument(
        '--images-sleep', type=int, default=-1,
        help='The time to sleep between importing images for importImages.php (negative for auto-calculation)',
    )
    parser.add_argument('wiki', help='Database name of the wiki to import to')

    args = parser.parse_args(input_args)
    if not args.wiki.endswith('wiki') and not args.wiki.endswith('wikibeta'):
        raise ValueError('<wiki> must be a proper database name')

    if not args.xml and not args.images:
        raise ValueError('--xml and/or --images must be passed')
    if args.images and not args.images_comment:
        raise ValueError('--images-comment must be passed when importing images')

    # This is honestly only for unit testing as I can't really think of a reason
    # to disable this check in production
    if check_paths:
        # This is not meant to be comprehensive, but just to make sure that someone
        # doesn't typo a path or so
        if args.xml and not os.path.exists(args.xml):
            raise ValueError(f'Cannot find XML to import: {repr(args.xml)}')
        if args.images and not os.path.exists(args.images):
            raise ValueError(f'Cannot find images to import: {repr(args.images)}')

    if args.images and args.images_sleep < 0:
        args.images_sleep = calculate_images_sleep(args.images) if check_paths else 0

    return args
  • File: miraheze/salt/mwcli/partial_reset_wiki.py (lines 24-29)
def main() -> None:
    parser = argparse.ArgumentParser(description='Executes the commands needed to reset wikis')
    parser.add_argument('--wiki', required=True, help='Old wiki database name')

    args = parser.parse_args()
    reset_wiki(args.wiki)
  • File: miraheze/salt/mwcli/rename_wiki.py (lines 25-31)
def main() -> None:
    parser = argparse.ArgumentParser(description='Executes the commands needed to rename wikis')
    parser.add_argument('--oldwiki', required=True, help='Old wiki database name')
    parser.add_argument('--newwiki', required=True, help='New wiki database name')

    args = parser.parse_args()
    rename_wiki(args.oldwiki, args.newwiki)

🧠 Learnings used
Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 2
File: miraheze/swift/fix_container_permissions.py:0-0
Timestamp: 2024-10-08T17:05:15.088Z
Learning: The `fix_container_permissions.py` script was updated to replace `os.system` calls with `subprocess.run` for enhanced security, and the unused `os` import was subsequently removed.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/mediawiki/mwscript.py:128-147
Timestamp: 2024-07-26T21:00:05.787Z
Learning: The `run` function in `miraheze/mediawiki/mwscript.py` can be enhanced by adding detailed error handling and retry mechanisms for executing external commands.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/mediawiki/mwscript.py:128-147
Timestamp: 2024-10-08T17:05:15.088Z
Learning: The `run` function in `miraheze/mediawiki/mwscript.py` can be enhanced by adding detailed error handling and retry mechanisms for executing external commands.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/puppet/disable_puppet.py:8-10
Timestamp: 2024-10-08T17:05:15.088Z
Learning: In the `disable_puppet.py` script, use `argparse` for argument parsing and `subprocess.run` for system calls to improve security and error handling.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/puppet/disable_puppet.py:8-10
Timestamp: 2024-07-26T21:00:05.787Z
Learning: In the `disable_puppet.py` script, use `argparse` for argument parsing and `subprocess.run` for system calls to improve security and error handling.

Learnt from: BlankEclair
Repo: miraheze/python-functions PR: 119
File: miraheze/mediawiki/mwimport.py:111-146
Timestamp: 2025-03-07T10:05:19.313Z
Learning: Miraheze's Python scripts use a simple "Type 'Y' to confirm" prompt pattern for user confirmation, consistent with existing implementations like in miraheze/mediawiki/mwscript.py.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/mediawiki/check_read_only.py:6-6
Timestamp: 2024-10-08T17:05:15.088Z
Learning: RhinosF1 prefers to keep `verify=False` for requests to localhost, considering it safe.

Learnt from: RhinosF1
Repo: miraheze/python-functions PR: 4
File: miraheze/mediawiki/check_read_only.py:6-6
Timestamp: 2024-07-26T21:00:05.787Z
Learning: RhinosF1 prefers to keep `verify=False` for requests to localhost, considering it safe.

💡 For code changes across multiple files, use @coderabbitai plan in an issue comment

Copy link
Contributor

@RhinosF1 RhinosF1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the rabbit

@Universal-Omega
Copy link
Member Author

@RhinosF1 I prefer we dont make it call mwscript directly like that until python-functions is fully deployed so this script can still be used independently as it is needed quite often.

@RhinosF1
Copy link
Contributor

@RhinosF1 I prefer we dont make it call mwscript directly like that until python-functions is fully deployed so this script can still be used independently as it is needed quite often.

The versions in puppet are the ones that are used until this is fully deployed so that won't impact

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants