Fix: Allow duplicate participant names across different workspaces#242
Fix: Allow duplicate participant names across different workspaces#242FloWuenne wants to merge 1 commit intoseqeralabs:mainfrom
Conversation
…es in same yml This commit fixes a bug where one could not specify the same participant or team name multiple times in the same yaml to add them to different workspaces.
adamrtalbot
left a comment
There was a problem hiding this comment.
I'm not massively following how this works so I think some clarity is required.
- Explain why this is needed
- How this works
- What problem this solves
| unique_identifiers = set() | ||
|
|
||
| # Define resources that can have duplicate names across different contexts | ||
| context_sensitive_resources = {"teams", "members", "participants"} |
There was a problem hiding this comment.
Maybe it's better to treat all resources as workspace specific? I think only dataset is left as one that must be globally unique within your organisation.
| if block_name in context_sensitive_resources: | ||
| # For context-sensitive resources, create a unique identifier | ||
| # that includes both name and workspace/organization context | ||
| workspace = find_context_value(cmd_args, ["--workspace", "--organization"]) | ||
| unique_id = f"{name}@{workspace}" if workspace else name | ||
| else: | ||
| # For other resources, use just the name as before | ||
| unique_id = name | ||
|
|
||
| if unique_id in unique_identifiers: | ||
| context_info = f" in context '{workspace}'" if block_name in context_sensitive_resources and workspace else "" |
There was a problem hiding this comment.
I'm not following the strategy. I think it's making a unique value by joining name and workspace into a string?
There was a problem hiding this comment.
I also think you're missing a value here:
| if block_name in context_sensitive_resources: | |
| # For context-sensitive resources, create a unique identifier | |
| # that includes both name and workspace/organization context | |
| workspace = find_context_value(cmd_args, ["--workspace", "--organization"]) | |
| unique_id = f"{name}@{workspace}" if workspace else name | |
| else: | |
| # For other resources, use just the name as before | |
| unique_id = name | |
| if unique_id in unique_identifiers: | |
| context_info = f" in context '{workspace}'" if block_name in context_sensitive_resources and workspace else "" | |
| if block_name in context_sensitive_resources: | |
| # For context-sensitive resources, create a unique identifier | |
| # that includes both name and workspace/organization context | |
| workspace = find_context_value(cmd_args, ["--workspace", "--organization"]) | |
| unique_id = f"{name}@{workspace}" if workspace else name | |
| else: | |
| # For other resources, use just the name as before | |
| unique_id = name | |
| if unique_id in unique_identifiers: | |
| context_info = f"{name} in context '{workspace}'" if block_name in context_sensitive_resources and workspace else "" |
| # For context-sensitive resources, create a unique identifier | ||
| # that includes both name and workspace/organization context | ||
| workspace = find_context_value(cmd_args, ["--workspace", "--organization"]) | ||
| unique_id = f"{name}@{workspace}" if workspace else name |
There was a problem hiding this comment.
Rather than concatenating a string, it might be helpful to make a unique ID object although this might be a lot more work:
@dataclass
class SeqeraResourceIdentifier:
name: str
workspace: str | None = NoneThen match that using a custom method:
def __eq__(self, other):
if self.workspace:
return self.name == other.name and self.workspace == other.workspace
else:
return self.name == other.nameThis encapsulates the caching logic into a reusable component without making flaky strings. Still, it's 100% premature optimisation.
This PR resolves issue where the same participant/team couldn't be added to multiple workspaces within a single YAML configuration file. Previously this would fail due to duplicate key handling - now properly supports adding the same user to different workspace contexts.
Thanks to Chris from Barnwell Bio for spotting this!
Copilot summary:
This pull request enhances the uniqueness validation logic in the
parse_yaml_blockfunction to correctly handle resources that may have duplicate names across different contexts (such as workspaces or organizations). It also introduces a new helper function to extract context values from command arguments.Improvements to uniqueness validation:
parse_yaml_blockto track unique combinations of resource name and context (workspace/organization) for context-sensitive resources (teams,members,participants), ensuring that duplicate names are only flagged within the same context.Helper function addition:
find_context_valueto extract context information (e.g., workspace or organization) from command arguments, supporting the enhanced uniqueness logic.