-
Notifications
You must be signed in to change notification settings - Fork 47
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.
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
fix: handle client generation in a dir containing multiple app spec types #599
Conversation
ccf0379
to
f457878
Compare
app_spec_path_or_dir: Path, | ||
output_path_pattern: str | None, | ||
*, | ||
raise_on_failure: bool, # TODO: NC - Maybe we should return the error instead? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps another option could be to remove raise_on_failure and add dataclass to represent result from generate_all response that will contain success and failure, something like
class GenerateAllResult:
successful_generations: list[tuple[Path, Path]] # List of (app_spec, output_path) that succeeded
failed_generations: list[tuple[Path, str]] # List of (app_spec, error_message) that failed
@property
def has_failures(self) -> bool:
"""Returns True if any generations failed."""
return len(self.failed_generations) > 0
and then the generate.py would be taking care of deciding when to raise an exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I was tossing up that option and forgot to remove the comment.
This approach would be behaviourally different though, as the exception is thrown on the first failure. It's tied into the fail_fast
option in the link
command, so we actually want to abort the generation on failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's actually a bit of a weird one, as it only handles path resolution failure, which is what the previous code was doing. I'm going to keep the boolean and rename to be more specific.
cfea328
to
a490b4f
Compare
Resolves #598