Skip to content

add logic for slot config #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zghp wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

add logic for slot config #44

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 24 additions & 2 deletions src/xts_core/plugins/xts_allocator_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ def _send_request(method, url, data=None) -> dict:
except requests.exceptions.RequestException as e:
plugin_utils.error(f'Error during request: {e}')


@staticmethod
def _format_slots_list_to_table(response:list[dict]) -> Table:
'''Format the list of slot dictionaries into a rich table
Expand Down Expand Up @@ -199,6 +198,7 @@ def _setup_allocator_args(self):
add_slot_parser.add_argument('--rack-name', required=True, help='Rack name of the slot.', dest='rack_name')
add_slot_parser.add_argument('--slot-name', required=True, help='Slot name.', dest='slot_name')
add_slot_parser.add_argument('--state', choices=['free', 'allocated'], default='free', help='State of the slot.')
add_slot_parser.add_argument('--config', required=False, help='path to a YAML or JSON slot config file.')

# update-slot
update_slot_parser= allocator_subparsers.add_parser('update-slot',
Expand Down Expand Up @@ -462,13 +462,31 @@ def _add_slot(self,
owner_email:str|None,
state:str='',
description:str='',
tags:list[str]=[]):
tags:list[str]=[],
config: str|None=None):
'''
Add a new slot to the allocator server.

Args:
args (list): Command-line arguments.
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

The docstring is outdated. It states "Args: args (list): Command-line arguments" but the function signature shows multiple specific parameters. Please update the docstring to document all parameters: server, rack_name, slot_name, platform, owner_email, state, description, tags, and the newly added config.

Suggested change
args (list): Command-line arguments.
server (str): Allocator server URL.
rack_name (str): Name of the rack.
slot_name (str): Name of the slot.
platform (str): Platform type for the slot.
owner_email (str or None): Email address of the slot owner.
state (str, optional): State of the slot. Defaults to ''.
description (str, optional): Description of the slot. Defaults to ''.
tags (list of str, optional): List of tags for the slot. Defaults to [].
config (str or None, optional): Path to a configuration file (YAML or JSON) for the slot. Defaults to None.

Copilot uses AI. Check for mistakes.
'''

configuration = None
if config:
import json, yaml
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

The yaml module is already imported at the top of the file (line 8), so it doesn't need to be imported again here. Only json needs to be imported locally. Consider moving the json import to the top of the file with other imports, or change this line to import json only.

Copilot uses AI. Check for mistakes.
try:
if config.endswith(('.yaml', '.yml')):
with open(config, 'r') as f:
configuration = yaml.safe_load(f)
elif config.endswith('.json'):
with open(config, 'r') as f:
configuration = json.load(f)
else:
plugin_utils.warning(f"Unsupported config format for file: {config}")
except Exception as e:
plugin_utils.error(f"Failed to load configuration file: {e}")
configuration = None
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

This line is unreachable code. The plugin_utils.error() function raises SystemExit(1) and terminates the program, so configuration = None will never be executed. Consider removing this line.

Suggested change
configuration = None

Copilot uses AI. Check for mistakes.
Comment on lines +486 to +488
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

Consider catching more specific exceptions instead of the broad Exception. For file operations, FileNotFoundError, PermissionError, and IOError are more appropriate. For parsing errors, yaml.YAMLError and json.JSONDecodeError should be caught separately to provide more specific error messages to the user.

Suggested change
except Exception as e:
plugin_utils.error(f"Failed to load configuration file: {e}")
configuration = None
except FileNotFoundError as e:
plugin_utils.error(f"Configuration file not found: {e}")
configuration = None
except PermissionError as e:
plugin_utils.error(f"Permission denied when accessing configuration file: {e}")
configuration = None
except IOError as e:
plugin_utils.error(f"I/O error when accessing configuration file: {e}")
configuration = None
except yaml.YAMLError as e:
plugin_utils.error(f"YAML parsing error in configuration file: {e}")
configuration = None
except json.JSONDecodeError as e:
plugin_utils.error(f"JSON parsing error in configuration file: {e}")
configuration = None

Copilot uses AI. Check for mistakes.

payload = {
'rackName': rack_name,
'slotName': slot_name,
Expand All @@ -478,6 +496,10 @@ def _add_slot(self,
'state': state,
'owner_email': owner_email,
}

if configuration:
payload['configuration'] = configuration

Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

Trailing whitespace detected. Please remove the trailing spaces at the end of this line.

Suggested change

Copilot uses AI. Check for mistakes.
Comment on lines +501 to +502
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

Consider validating that the loaded configuration is a dictionary before adding it to the payload. Both yaml.safe_load() and json.load() can return various types (None, lists, strings, etc.) depending on the file content. If the configuration is not a dictionary, it could cause issues with the API endpoint.

Suggested change
payload['configuration'] = configuration
if isinstance(configuration, dict):
payload['configuration'] = configuration
else:
plugin_utils.warning(f"Configuration loaded from {config} is not a dictionary and will be ignored.")

Copilot uses AI. Check for mistakes.
response = self._send_request('POST', f'{server}/add_slot', payload)
if response:
plugin_utils.info(response.get('message'))
Expand Down
Loading