Skip to content
Open
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
29 changes: 29 additions & 0 deletions tools/policy_and_auth/login_requests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import sys
import os
import logging

_this_dir = os.path.dirname(os.path.abspath(__file__))
path_to_dspace_lib = os.path.join(_this_dir, "../../libs/dspace-rest-python")
sys.path.insert(0, path_to_dspace_lib)
sys.path.insert(0, os.path.join(_this_dir, "../../src"))
import dspace # noqa
import settings # noqa
import project_settings # noqa
from utils import init_logging, update_settings # noqa
_logger = logging.getLogger()

env = update_settings(settings.env, project_settings.settings)
init_logging(_logger, env["log_file"])
Comment on lines +15 to +16
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation for required environment variables.

The script assumes required credentials exist in the environment but doesn't validate them. This could lead to unclear error messages if configuration is missing.

 env = update_settings(settings.env, project_settings.settings)
 init_logging(_logger, env["log_file"])

+# Validate required environment variables
+required_keys = ["backend.endpoint", "backend.user", "backend.password"]
+missing_keys = []
+for key in required_keys:
+    keys = key.split('.')
+    value = env
+    try:
+        for k in keys:
+            value = value[k]
+        if not value:
+            missing_keys.append(key)
+    except (KeyError, TypeError):
+        missing_keys.append(key)
+
+if missing_keys:
+    _logger.error(f"Missing required environment variables: {missing_keys}")
+    sys.exit(1)

Also applies to: 21-21

🤖 Prompt for AI Agents
In tools/policy_and_auth/login_requests.py at lines 15-16 and line 21, the code
uses environment variables without validating their presence, which can cause
unclear errors. Add explicit checks after loading the environment to verify that
all required environment variables are set. If any are missing, raise a clear
exception or log an error indicating which variables are absent before
proceeding with the rest of the script.


if __name__ == "__main__":
_logger.info("Started...")

dspace_be = dspace.rest(env["backend"]["endpoint"], env["backend"]["user"], env["backend"]["password"], True)
for i in range(1, 10):
_logger.info(f"Authenticating {i}")
dspace_be.client.authenticate()
Comment on lines +21 to +24
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling and make the loop count configurable.

The authentication loop lacks error handling and uses a hard-coded count. This could cause the script to fail silently or make it difficult to customize for different testing scenarios.

-    dspace_be = dspace.rest(env["backend"]["endpoint"], env["backend"]["user"], env["backend"]["password"], True)
-    for i in range(1, 10):
-        _logger.info(f"Authenticating {i}")
-        dspace_be.client.authenticate()
+    try:
+        dspace_be = dspace.rest(env["backend"]["endpoint"], env["backend"]["user"], env["backend"]["password"], True)
+        
+        # Make loop count configurable via environment variable
+        loop_count = int(os.environ.get("AUTH_LOOP_COUNT", "9"))
+        
+        for i in range(1, loop_count + 1):
+            _logger.info(f"Authenticating attempt {i}/{loop_count}")
+            try:
+                dspace_be.client.authenticate()
+                _logger.info(f"Authentication attempt {i} successful")
+            except Exception as e:
+                _logger.error(f"Authentication attempt {i} failed: {e}")
+                
+    except Exception as e:
+        _logger.error(f"Failed to initialize DSpace backend: {e}")
+        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
dspace_be = dspace.rest(env["backend"]["endpoint"], env["backend"]["user"], env["backend"]["password"], True)
for i in range(1, 10):
_logger.info(f"Authenticating {i}")
dspace_be.client.authenticate()
try:
dspace_be = dspace.rest(
env["backend"]["endpoint"],
env["backend"]["user"],
env["backend"]["password"],
True
)
# Make loop count configurable via environment variable
loop_count = int(os.environ.get("AUTH_LOOP_COUNT", "9"))
for i in range(1, loop_count + 1):
_logger.info(f"Authenticating attempt {i}/{loop_count}")
try:
dspace_be.client.authenticate()
_logger.info(f"Authentication attempt {i} successful")
except Exception as e:
_logger.error(f"Authentication attempt {i} failed: {e}")
except Exception as e:
_logger.error(f"Failed to initialize DSpace backend: {e}")
sys.exit(1)
🤖 Prompt for AI Agents
In tools/policy_and_auth/login_requests.py around lines 21 to 24, the
authentication loop uses a hard-coded count of 9 and lacks error handling.
Modify the code to make the loop count configurable via a parameter or
environment variable, and wrap the authentication call in a try-except block to
catch and log any exceptions during authentication attempts.


# Call logout every 5th request
# if i % 5 == 0:
# dspace_be.client.logout()
# print("Logged out")