Skip to content

Return error when instance added to multiple fleets(#1699)#1938

Merged
r4victor merged 2 commits intodstackai:masterfrom
swsvc:master
Nov 1, 2024
Merged

Return error when instance added to multiple fleets(#1699)#1938
r4victor merged 2 commits intodstackai:masterfrom
swsvc:master

Conversation

@swsvc
Copy link
Copy Markdown
Contributor

@swsvc swsvc commented Oct 31, 2024

No description provided.

Comment on lines +160 to +162
raise ConfigurationValidationError(
detail=f"Instances [{', '.join(instances_already_in_fleet)}] are already assigned to a fleet."
)
Copy link
Copy Markdown
Collaborator

@r4victor r4victor Nov 1, 2024

Choose a reason for hiding this comment

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

Please raise ServerClientError("error message"). It will be converted to a 400 response and handled correctly by the CLI. There is no need to introduce new errors.

existing_hosts.add(instance_conn_info.host)

instances_already_in_fleet = []
for new_instance in spec.configuration.ssh_config.hosts:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ssh_config.hosts may store hostnames but it may also store structs containing a hostname. You need to handle the second case.

hosts: Annotated[
List[Union[SSHHostParams, str]],
Field(
description="The per host connection parameters: a hostname or an object that overrides default ssh parameters"
),
]

) -> FleetPlan:
# TODO: refactor offers logic into a separate module to avoid depending on runs

if spec.configuration.ssh_config and spec.configuration.ssh_config.hosts:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The entire check should be moved into a separate function like _check_ssh_hosts_not_yet_added()

@swsvc
Copy link
Copy Markdown
Contributor Author

swsvc commented Nov 1, 2024

Done, please review fixes

@r4victor
Copy link
Copy Markdown
Collaborator

r4victor commented Nov 1, 2024

@swsvc, merged, thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants