Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions scripts/constant.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
MIGRATE_V211_LIMITS = NETWORK_STORE_SERVER_URL + "/migration/v211limits/{networkId}/{variantNum}"
MIGRATE_V214_TAP_CHANGER_STEPS = NETWORK_STORE_SERVER_URL + "/migration/v214tapChangeSteps/{networkId}/{variantNum}"
MIGRATE_V221_LIMITS = NETWORK_STORE_SERVER_URL + "/migration/v221limits/{networkId}/{variantNum}"
MIGRATE_V237_LIMITS = NETWORK_STORE_SERVER_URL + "/migration/v237limits/{networkId}/{variantNum}"

DELETE_NETWORKS = NETWORK_STORE_SERVER_URL + "/networks"

Expand Down
5 changes: 5 additions & 0 deletions scripts/functions/networks/networks.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,8 @@ def migrate_v214_tapchangersteps(network_id, variant_num):
def migrate_v221_limits(network_id, variant_num):
response = requests.put(constant.MIGRATE_V221_LIMITS.format(networkId = network_id, variantNum = variant_num))
response.raise_for_status()


def migrate_v237_limits(network_id, variant_num):
response = requests.put(constant.MIGRATE_V237_LIMITS.format(networkId=network_id, variantNum=variant_num))
response.raise_for_status()
Comment on lines +33 to +35
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# Locate and examine the networks.py file
find . -path "*scripts/functions/networks/networks.py" -type f

Repository: gridsuite/admin-tools

Length of output: 106


🏁 Script executed:

# Show the content around lines 33-35
if [ -f "scripts/functions/networks/networks.py" ]; then
  cat -n "scripts/functions/networks/networks.py" | sed -n '25,45p'
fi

Repository: gridsuite/admin-tools

Length of output: 697


🏁 Script executed:

# Search for where migrate_v237_limits is called
rg "migrate_v237_limits" -n -B 2 -A 2

Repository: gridsuite/admin-tools

Length of output: 1293


🏁 Script executed:

# Check for timeout patterns in similar requests in the codebase
rg "requests\.(put|post|get|patch)" -A 2 | head -100

Repository: gridsuite/admin-tools

Length of output: 7742


🏁 Script executed:

# Look for timeout constants defined
rg "TIMEOUT|timeout" scripts/functions/networks/ -n

Repository: gridsuite/admin-tools

Length of output: 47


🏁 Script executed:

# Examine the migrate_v237_limits.py file to see the full loop context
cat -n "scripts/migrate_v237_limits.py" | sed -n '60,75p'

Repository: gridsuite/admin-tools

Length of output: 1082


🏁 Script executed:

# Check for any timeout configurations or constants
rg "timeout" -i --max-count 20

Repository: gridsuite/admin-tools

Length of output: 3227


🏁 Script executed:

# Check similar migration functions to see if any have timeouts
cat -n "scripts/functions/networks/networks.py" | head -40

Repository: gridsuite/admin-tools

Length of output: 1682


Add an explicit timeout to prevent indefinite blocking in the migration loop.

This function is called within a nested loop over all networks and variants. Without a timeout, a single hung HTTP request will stall the entire migration indefinitely. The fix is straightforward.

Suggested fix
 def migrate_v237_limits(network_id, variant_num):
-    response = requests.put(constant.MIGRATE_V237_LIMITS.format(networkId=network_id, variantNum=variant_num))
+    response = requests.put(
+        constant.MIGRATE_V237_LIMITS.format(networkId=network_id, variantNum=variant_num),
+        timeout=30,
+    )
     response.raise_for_status()
🧰 Tools
🪛 Ruff (0.15.13)

[error] 34-34: Probable use of requests call without timeout

(S113)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/functions/networks/networks.py` around lines 33 - 35, The
migrate_v237_limits function currently issues requests.put without a timeout
which can hang the migration loop; modify migrate_v237_limits to pass an
explicit timeout to requests.put (e.g., timeout=10 or a small configurable
constant) so the call fails fast, and keep the existing
response.raise_for_status() behavior; reference the requests.put call and
constant.MIGRATE_V237_LIMITS when making this change and optionally catch
requests.exceptions.Timeout around the call if you want to log or retry
separately.

77 changes: 77 additions & 0 deletions scripts/migrate_v237_limits.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
#
# Copyright (c) 2026, RTE (http://www.rte-france.com)
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
#

import requests
import argparse
import constant
import sys
from tqdm import tqdm

from functions.networks.networks import get_all_networks_uuid
from functions.networks.networks import get_variants
from functions.networks.networks import migrate_v237_limits
from functions.plateform.plateform import get_plateform_info
from functions.plateform.plateform import check_server_status

#
# @author Etienne Lesot <etienne.lesot at rte-france.com>
#

parser = argparse.ArgumentParser(description='Send requests to the gridsuite services to migrate V2.37 limits', )

parser.add_argument("-n", "--dry-run", help="test mode (default) will not execute any deletion request",
action="store_true")


args = parser.parse_args()
dry_run = args.dry_run
Comment on lines +24 to +31
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make dry-run the real default before this runs against all networks.

With action="store_true", running the script with no flags sets dry_run to False, so it will execute the migration even though the help text says dry-run is the default. The help text also mentions deletion requests, but this script issues migration PUTs.

Suggested fix
 parser = argparse.ArgumentParser(description='Send requests to the gridsuite services to migrate V2.37 limits', )
 
-parser.add_argument("-n", "--dry-run", help="test mode (default) will not execute any deletion request",
-                    action="store_true")
+parser.add_argument(
+    "-n",
+    "--dry-run",
+    dest="dry_run",
+    action="store_true",
+    default=True,
+    help="test mode (default) will not execute any migration request",
+)
+parser.add_argument(
+    "--execute",
+    dest="dry_run",
+    action="store_false",
+    help="apply the migration requests",
+)
 
 
 args = parser.parse_args()
 dry_run = args.dry_run
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/migrate_v237_limits.py` around lines 24 - 31, The current
parser.add_argument for "-n/--dry-run" uses action="store_true" so dry_run
becomes False by default; change the CLI so dry_run defaults to True and the
flag explicitly enables execution (e.g., add a new flag like "--execute" / "-x"
with action="store_true" or change to "--no-dry-run" with action="store_true"
and then set dry_run = not args.execute/args.no_dry_run), update the dry_run
assignment accordingly, and update the help text on parser.add_argument and the
description to state that the script performs migration PUTs (not deletions) and
that dry-run is the default unless the execute/no-dry-run flag is provided.


print("---------------------------------------------------------")
print("V2.37 limits migration script")
if dry_run:
print("dry-run=" + str(dry_run) + " -> will run without modifying anything (test mode)")
if constant.DEV:
print("DEV=" + str(constant.DEV) + " -> hostnames configured for a local execution (172.17.0.1:xxxx)")
print("\n")

# Check network-store-server
if not check_server_status(constant.NETWORK_STORE_SERVER_HOSTNAME): sys.exit()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Exit non-zero when the preflight check fails.

A bare sys.exit() returns success here, so CI or batch runners can treat “network-store-server unavailable” as a successful migration run.

Suggested fix
-if not check_server_status(constant.NETWORK_STORE_SERVER_HOSTNAME): sys.exit()
+if not check_server_status(constant.NETWORK_STORE_SERVER_HOSTNAME):
+    sys.exit(1)
📝 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.

Suggested change
if not check_server_status(constant.NETWORK_STORE_SERVER_HOSTNAME): sys.exit()
if not check_server_status(constant.NETWORK_STORE_SERVER_HOSTNAME):
sys.exit(1)
🧰 Tools
🪛 Ruff (0.15.13)

[error] 42-42: Multiple statements on one line (colon)

(E701)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/migrate_v237_limits.py` at line 42, The preflight check uses
sys.exit() which returns zero on failure; update the failure path where
check_server_status(constant.NETWORK_STORE_SERVER_HOSTNAME) is False to exit
with a non-zero code (e.g., sys.exit(1)) and optionally emit an error message
before exiting so CI/batch runners treat the failure as an error; modify the
line referencing check_server_status and sys.exit() accordingly.

print("\n")
# Just getting an enlightening url opportunistically from here because it exists
plateformName = get_plateform_info()['redirect_uri']

print("\n")

print("---------------------------------------------------------")
print("This script will apply on plateform = " + plateformName )
print("\n")
print("===> network-store-server seems OK ! The script can proceed")
print("\n")
networks = get_all_networks_uuid()
print("For a total of " + str(len(networks)) + " networks")
print("---------------------------------------------------------")

print("V2.11 limits migration (dry-run=" + str(dry_run) + ") in processing...")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix the migration banner version.

This still logs V2.11 limits migration, which is easy to confuse with the older migration during operations.

Suggested fix
-print("V2.11 limits migration (dry-run=" + str(dry_run) + ") in processing...")
+print("V2.37 limits migration (dry-run=" + str(dry_run) + ") in processing...")
📝 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.

Suggested change
print("V2.11 limits migration (dry-run=" + str(dry_run) + ") in processing...")
print("V2.37 limits migration (dry-run=" + str(dry_run) + ") in processing...")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/migrate_v237_limits.py` at line 58, The migration banner prints the
wrong version; update the print call in migrate_v237_limits.py that currently
uses "V2.11 limits migration (dry-run=" + str(dry_run) + ")" so it reflects the
correct migration version for this script (e.g., "V2.37 limits migration
(dry-run=" + str(dry_run) + ")"), keeping the dry_run variable interpolation
unchanged to avoid altering behavior.

failCount = 0
successCount = 0
for network in tqdm(networks):
variants = get_variants(network['uuid'])
for variant in variants:
if not dry_run:
try:
migrate_v237_limits(network['uuid'], variant['num'])
successCount += 1
except Exception as e:
failCount += 1
# print only str(e) instead of the full traceback because we call this method from a simple for loop script
tqdm.write("network " + network['uuid'] + ", variantNum " + str(variant['num']) + " => migration failed: "+ str(e))
if isinstance(e, requests.exceptions.RequestException) and e.response is not None:
tqdm.write("Response body: " + repr(e.response.text)) # repr for cheap escaping
tqdm.write("") # emtpy newline between errors for legibility
Comment on lines +68 to +74
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

cat -n scripts/migrate_v237_limits.py | head -100

Repository: gridsuite/admin-tools

Length of output: 3740


🏁 Script executed:

find . -type f -name "*.py" | xargs grep -l "def migrate_v237_limits"

Repository: gridsuite/admin-tools

Length of output: 106


🏁 Script executed:

cat -n ./scripts/functions/networks/networks.py | head -200

Repository: gridsuite/admin-tools

Length of output: 1682


Narrow exception handling to requests failures only.

The migrate_v237_limits() function only raises requests.exceptions.RequestException and its subclasses (from requests.put() and response.raise_for_status()). Catching all Exception masks programming bugs (e.g., KeyError from malformed API data) that should fail fast rather than continue with partial migrations.

Suggested fix
-            except Exception as e:
+            except requests.exceptions.RequestException as e:
                 failCount += 1
                 # print only str(e) instead of the full traceback because we call this method from a simple for loop script
                 tqdm.write("network " + network['uuid'] + ", variantNum " + str(variant['num']) + " => migration failed: "+ str(e))
-                if isinstance(e, requests.exceptions.RequestException) and e.response is not None:
+                if e.response is not None:
                     tqdm.write("Response body: " + repr(e.response.text)) # repr for cheap escaping
                 tqdm.write("") # emtpy newline between errors for legibility
🧰 Tools
🪛 Ruff (0.15.13)

[warning] 68-68: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/migrate_v237_limits.py` around lines 68 - 74, The except block in
migrate_v237_limits() is too broad and swallows programming errors; change the
handler to only catch requests.exceptions.RequestException (e.g., replace
"except Exception as e:" with "except requests.exceptions.RequestException as
e:") so that network/HTTP failures are logged for network (network['uuid']) and
variant (variant['num']) as currently done, including the response body when
available, while allowing non-request exceptions (KeyError, TypeError, etc.) to
propagate and fail fast.

print("End of V2.37.0 limits migration")
print("Variant migration sucesses : " + str(successCount))
print("Variant migration failures : " + str(failCount))
Loading