Fix TypeError in _AllowedDomainNames.valid_name#5048
Conversation
Up to standards ✅🟢 Issues
|
… class Agent-Logs-Url: https://github.com/ansys/pyfluent/sessions/8d05b3e3-5aca-472f-a55f-b9cc5adf0393 Co-authored-by: Gobot1234 <50501825+Gobot1234@users.noreply.github.com>
_AllowedDomainNames.valid_name
There was a problem hiding this comment.
Pull request overview
This PR fixes runtime validation of invalid domain names in the solution variables service by replacing an incorrect ZoneError(domain_name=...) raise (which caused a TypeError) with a dedicated DomainError that formats the intended allowed-name message.
Changes:
- Introduced
DomainError(ValueError)for invalid domain-name contexts. - Updated
_AllowedDomainNames.valid_nameto raiseDomainErrorinstead of misusingZoneError. - Updated the corresponding docstring in
_AllowedDomainNames.valid_name.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not self.is_valid(domain_name): | ||
| raise ZoneError( | ||
| raise DomainError( | ||
| domain_name=domain_name, | ||
| allowed_values=self(), | ||
| ) |
There was a problem hiding this comment.
Consider adding a regression test for this behavior (e.g., calling SolutionVariableInfo.get_variables_info(..., domain_name="bad")) asserting that an invalid domain now raises DomainError (and that the message includes the allowed domain list). This would prevent the prior TypeError regression from reappearing.
…er PR #5048) Agent-Logs-Url: https://github.com/ansys/pyfluent/sessions/7cf80380-e40d-4804-b766-bab5a89d01f4 Co-authored-by: Gobot1234 <50501825+Gobot1234@users.noreply.github.com>
|
Closing this in favour of #5050 |
_AllowedDomainNames.valid_nameraised aTypeErrorat runtime because it calledZoneError(domain_name=..., ...), butZoneError.__init__only acceptszone_nameas its identifier parameter.Context
ZoneErrorsignature is__init__(self, zone_name: str, allowed_values: List[str]). Passingdomain_name=as a keyword argument always fails withTypeError: unexpected keyword argument 'domain_name', making any invalid domain name produce a confusing secondary error instead of the intended validation message.Change Summary
DomainError(ValueError)class mirroringZoneErrorbut typed for domain context (domain_nameparameter,"domain"error context string)_AllowedDomainNames.valid_nameto raiseDomainErrorinstead ofZoneErrorRationale
Creating a dedicated
DomainErroris cleaner than patchingZoneErrorto accept both parameter names, keeps error types semantically distinct (callers can catchDomainErrorvsZoneErrorindependently), and matches the existing pattern used byInvalidSolutionVariableNameError.Impact
_AllowedDomainNames.valid_name— previously always threwTypeErroron an invalid domain name; now correctly raisesDomainErrorwith an informative message listing allowed domain names.