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.

Sign up for GitHub

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

Jump to bottom

fix(parse-dsl): fix issue with infinite loops triggered when parsing some models #76

Merged
merged 4 commits into from
Oct 25, 2022
Merged

fix(parse-dsl): fix issue with infinite loops triggered when parsing some models #76

merged 4 commits into from
Oct 25, 2022

Conversation

adriantam
Copy link
Member

@adriantam adriantam commented Oct 21, 2022 •
edited by rhamzeh
Loading

Description

We had some issues in our grammar causing infinite loops when trying to parse, especially when extraneous spaces or newlines were found.

We still have some issues with parsing comments, but it has been limited.

References

Closes #77

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

@adriantam adriantam requested a review from a team as a code owner October 21, 2022 18:46
@adriantam adriantam marked this pull request as draft October 21, 2022 18:46
@adriantam adriantam force-pushed the fix/add_parsedsl_test branch 2 times, most recently from 2b6ec8a to d27bf89 Compare October 21, 2022 19:23
@adriantam adriantam force-pushed the fix/add_parsedsl_test branch from d27bf89 to f13f60b Compare October 21, 2022 19:28
@adriantam
chore: debug parseDSL
d66d838 
Able to reproduce a case that takes a long time to parse
@rhamzeh rhamzeh added this to the v0.0.7 milestone Oct 22, 2022
@adriantam adriantam force-pushed the fix/add_parsedsl_test branch from d66d838 to c74b34a Compare October 24, 2022 13:30
@rhamzeh rhamzeh force-pushed the fix/add_parsedsl_test branch from c74b34a to c9b71fe Compare October 25, 2022 14:47
@rhamzeh rhamzeh changed the title fix: adding test for parseDSL with spaces fix(parse-dsl): fix issue with infinite loops triggered when parsing some models with extraneous spaces Oct 25, 2022
@rhamzeh rhamzeh changed the title fix(parse-dsl): fix issue with infinite loops triggered when parsing some models with extraneous spaces fix(parse-dsl): fix issue with infinite loops triggered when parsing some models Oct 25, 2022
@rhamzeh rhamzeh force-pushed the fix/add_parsedsl_test branch from c9b71fe to b57ea16 Compare October 25, 2022 14:49
@rhamzeh rhamzeh linked an issue Oct 25, 2022 that may be closed by this pull request
Syntax Transformer freezes and takes a long time to respond under certain models #77
Closed
@rhamzeh rhamzeh marked this pull request as ready for review October 25, 2022 14:52
@rhamzeh
Copy link
Member

rhamzeh commented Oct 25, 2022 •
edited
Loading

@adriantam this is ready for review

@rhamzeh rhamzeh self-assigned this Oct 25, 2022
@adriantam
Copy link
Member Author

Only question is whether we want to allow comment before "relations" - and we can defer this to later PR. Other than that, LGTM.

@rhamzeh
Copy link
Member

rhamzeh commented Oct 25, 2022

Only question is whether we want to allow comment before "relations" - and we can defer this to later PR. Other than that, LGTM.

Comments before relations won't be allowed, but there should be one before define - however it is also triggering the infinite loop, so removed it for now.

@rhamzeh rhamzeh merged commit c7629a6 into main Oct 25, 2022
@rhamzeh rhamzeh deleted the fix/add_parsedsl_test branch October 25, 2022 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rhamzeh rhamzeh rhamzeh approved these changes

Assignees

@rhamzeh rhamzeh

Labels
None yet
Projects
None yet
Milestone
v0.0.7
Development

Successfully merging this pull request may close these issues.

Syntax Transformer freezes and takes a long time to respond under certain models
2 participants
@adriantam @rhamzeh