Skip to content
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

Fail-fast mode #748

Closed
senier opened this issue Aug 23, 2021 · 3 comments · Fixed by #749
Closed

Fail-fast mode #748

senier opened this issue Aug 23, 2021 · 3 comments · Fixed by #749
Assignees
Labels
small Effort of one person-day or less

Comments

@senier
Copy link
Member

senier commented Aug 23, 2021

Despite the improvements in #625, it can still take significant time to check a specification, even with verification turned off. One reason may be that we collect as many errors as reasonable before stopping. When refining a specification interactively it may be preferable to stop on the first error.

Add a command line option for fail as early as possible and change the RecordFluxError class to immediately raise and exception should a respective global flag be set.

@senier senier created this issue from a note in RecordFlux 0.6 (To do) Aug 23, 2021
@senier senier added enhancement small Effort of one person-day or less labels Aug 23, 2021
@senier senier self-assigned this Aug 23, 2021
@treiher
Copy link
Collaborator

treiher commented Aug 23, 2021

An implementation, which raises an exception as soon as an entry to RecordFluxError is added, could lead to loss of information in several cases. For example:

                            self.error.append(
                                f'conflicting conditions for field "{f.name}"',
                                Subsystem.MODEL,
                                Severity.ERROR,
                                f.identifier.location,
                            )
                            self.error.append(
                                f"condition {i1} ({f.identifier} -> {c1.target.identifier}):"
                                f" {c1_message}",
                                Subsystem.MODEL,
                                Severity.INFO,
                                c1.condition.location,
                            )
                            self.error.append(
                                f"condition {i2} ({f.identifier} -> {c2.target.identifier}):"
                                f" {c2_message}",
                                Subsystem.MODEL,
                                Severity.INFO,
                                c2.condition.location,
                            )

The use of extend instead of multiple append at the affected places should prevent that.

@senier
Copy link
Member Author

senier commented Aug 23, 2021

The use of extend instead of multiple append at the affected places should prevent that.

What's a bit annoying is that the author of those errors must always be aware of that issue. Maybe we should only support extend (even though this complicates writing a single-error statement)?

@treiher
Copy link
Collaborator

treiher commented Aug 23, 2021

Yes, it would be better if the author would be forced to do the right thing. Only supporting extend would at least prevent cases like the example shown above. I'm not sure how effective this would be in other cases. But probably it is better than nothing. Having a bit more complicated single-error statements seems acceptable to me.

@senier senier moved this from To do to In progress in RecordFlux 0.6 Aug 24, 2021
senier pushed a commit that referenced this issue Aug 24, 2021
senier pushed a commit that referenced this issue Aug 24, 2021
@senier senier moved this from In progress to Under review in RecordFlux 0.6 Aug 24, 2021
@treiher treiher linked a pull request Aug 24, 2021 that will close this issue
senier pushed a commit that referenced this issue Aug 24, 2021
senier pushed a commit that referenced this issue Aug 24, 2021
senier pushed a commit that referenced this issue Aug 24, 2021
senier pushed a commit that referenced this issue Aug 25, 2021
senier pushed a commit that referenced this issue Aug 25, 2021
senier pushed a commit that referenced this issue Aug 25, 2021
RecordFlux 0.6 automation moved this from Under review to Merged Aug 25, 2021
senier pushed a commit that referenced this issue Aug 25, 2021
senier pushed a commit that referenced this issue Aug 25, 2021
senier pushed a commit that referenced this issue Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
small Effort of one person-day or less
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants