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

fix: Remove new types from in-memory cache on generate error #122

Merged
merged 2 commits into from
Jan 27, 2022

Conversation

AndrewSisley
Copy link
Contributor

Closes #104

Previously a test would pass if an error was returned due to early exit
Previously they would hang around until database restart - corrupting the schema and preventing schema correction
@AndrewSisley AndrewSisley added bug Something isn't working area/schema Related to the schema system labels Jan 18, 2022
@AndrewSisley AndrewSisley self-assigned this Jan 18, 2022
@codecov
Copy link

codecov bot commented Jan 18, 2022

Codecov Report

Merging #122 (978d6a4) into develop (0cd7370) will increase coverage by 0.25%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #122      +/-   ##
===========================================
+ Coverage    54.03%   54.28%   +0.25%     
===========================================
  Files           35       35              
  Lines         3633     3653      +20     
===========================================
+ Hits          1963     1983      +20     
  Misses        1441     1441              
  Partials       229      229              
Impacted Files Coverage Δ
query/graphql/schema/generate.go 82.02% <100.00%> (+0.69%) ⬆️
query/graphql/schema/relations.go 54.63% <0.00%> (ø)

Copy link
Member

@jsimnz jsimnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great fix. There will def be more to overhaul once schema migrations come in. But for now this looks like it works great. The added failure test to check for invalid states is awesome!

@AndrewSisley AndrewSisley merged commit 4652589 into develop Jan 27, 2022
@AndrewSisley AndrewSisley deleted the sisley/fix/I104-generator-errors-clear-schema branch January 27, 2022 12:38
@orpheuslummis orpheuslummis mentioned this pull request Jan 29, 2022
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
…etwork#122)

* Force checking of returned errors in generator tests

Previously a test would pass if an error was returned due to early exit

* Remove new types from IM cache on generate error

Previously they would hang around until database restart - corrupting the schema and preventing schema correction
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/schema Related to the schema system bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Attempting to add a schema with an invalid datatype results in an invalid database schema
2 participants