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

Add opt to remove TypeInstance's revision, fix update mutation #698

Merged
merged 2 commits into from
Apr 11, 2022

Conversation

mszostok
Copy link
Member

@mszostok mszostok commented Apr 6, 2022

Description

Changes proposed in this pull request:

  • Add opt to remove TypeInstance's revision via dedicated gRPC method
  • Store new revision in external storage before executing update cypher query
  • Rollback stored revision in case of failed transaction

The Local Hub code was simplified thanks to handling the update logic directly in mutation resolver instead of value field resolver. However, the additionall logic (rpc DeleteRevision) is now required to be implemented on each storage backend.

Testing

I added e2e tests for Local Hub to prove that the contract was not broken with the new approach.

Related issue(s)

Fix #661

@mszostok mszostok added bug Something isn't working area/hub Relates to Hub labels Apr 6, 2022
@mszostok mszostok force-pushed the fix-hub-update branch 4 times, most recently from 0f9d7c7 to 131377c Compare April 6, 2022 11:18
@mkuziemko mkuziemko self-assigned this Apr 7, 2022
Copy link

@mkuziemko mkuziemko left a comment

Choose a reason for hiding this comment

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

I verified it locally and it works well 👍 small comments only

test/local-hub/external_storage_backend_test.go Outdated Show resolved Hide resolved
internal/secret-storage-backend/server.go Show resolved Hide resolved
@mszostok mszostok force-pushed the fix-hub-update branch 2 times, most recently from 0a4f933 to 31fe8f5 Compare April 11, 2022 07:20
Copy link

@mkuziemko mkuziemko left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@mszostok mszostok merged commit c0d2515 into capactio:main Apr 11, 2022
@mszostok mszostok deleted the fix-hub-update branch April 11, 2022 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hub Relates to Hub bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Always store the value property on update mutation
2 participants