Skip to content

Conversation

@rakeshkky
Copy link
Member

@rakeshkky rakeshkky commented Nov 4, 2020

Description

This is an incremental PR towards #5797. It introduces an abstraction for metadata storage MonadMetadataStorage so that the caller of the function runHGEServer can customize the implementations via type class instance. It is a step towards separation of metadata from users database. This PR ONLY abstracted the operations related to scheduled triggers eventing sub-system. Subsequent PRs will contain operations related to metadata management and async actions sub system.

This also includes moving of types related to scheduled triggers.

To Reviewers: Feel free to post your thoughts on type class design and suggest improvements 😅

Changelog

  • CHANGELOG.md is updated with user-facing content relevant to this PR. If no changelog is required, then add the no-changelog-required label.

Affected components

  • Server

Related Issues

Solution and Design

Each function in the new type class operates in MetadataStorageT m a monad transformer where m is base App monad in which the function runHGEServer operates in.

Steps to test and verify

Limitations, known bugs & workarounds

Server checklist

Catalog upgrade

Does this PR change Hasura Catalog version?

  • No
  • Yes

Metadata

Does this PR add a new Metadata feature?

  • No
  • Yes

GraphQL

  • No new GraphQL schema is generated
  • New GraphQL schema is being generated

Breaking changes

  • No Breaking changes
  • There are breaking changes

@rakeshkky rakeshkky requested a review from abooij November 4, 2020 14:46
@rakeshkky rakeshkky requested a review from abooij November 5, 2020 14:05
@rakeshkky rakeshkky requested a review from lexi-lambda November 5, 2020 14:16
@netlify
Copy link

netlify bot commented Nov 6, 2020

Deploy preview for hasura-docs ready!

Built with commit efbca69

https://deploy-preview-6131--hasura-docs.netlify.app

This partially reverts commit efbca69.

The reviewer's suggestion did not work out
@netlify
Copy link

netlify bot commented Nov 9, 2020

Deploy preview for hasura-docs ready!

Built with commit 7227d54

https://deploy-preview-6131--hasura-docs.netlify.app

@rakeshkky rakeshkky requested a review from abooij November 10, 2020 08:35
@rakeshkky rakeshkky force-pushed the 5797-scheduled-triggers-abstraction branch from add6ad7 to 2b25700 Compare November 10, 2020 08:41
@rakeshkky rakeshkky force-pushed the 5797-scheduled-triggers-abstraction branch from 2b25700 to 37db141 Compare November 10, 2020 09:03
Resolve Conflicts:
	server/src-lib/Hasura/Backends/Postgres/SQL/DML.hs
@kodiakhq
Copy link
Contributor

kodiakhq bot commented Nov 11, 2020

This PR currently has a merge conflict. Please resolve this and then re-add the auto-update-auto-merge label.

-> Each method in the aforementioned type class is executed in a
separate transaction. Using specific function `runInSeparateTx` instead
of `liftTx` is reasonable.
Copy link
Contributor

@abooij abooij left a comment

Choose a reason for hiding this comment

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

Approving this PR to unblock further work, despite ongoing discussions that we may wish to return to at a later stage.

Copy link
Contributor

@lexi-lambda lexi-lambda left a comment

Choose a reason for hiding this comment

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

After my conversation last night with @rakeshkky, I understand the issues at play here much better. I’ve left a suggestion to reword the comment to be more explicit about exactly what’s going on, but otherwise I’m happy with this PR as it stands

@hasura-bot
Copy link
Contributor

Thanks for your contribution! Your changes have been merged successfully.

@hasura-bot hasura-bot closed this Nov 25, 2020
hasura-bot added a commit that referenced this pull request Nov 25, 2020
An incremental PR towards #5797

* metadata storage abstraction for scheduled triggers

Co-authored-by: rakeshkky <[email protected]>
Co-authored-by: Rakesh Emmadi <[email protected]>
Co-authored-by: Auke Booij <[email protected]>
GITHUB_PR_NUMBER: 6131
GITHUB_PR_URL: #6131

* update pro server code

Co-authored-by: rakeshkky <[email protected]>
Co-authored-by: Auke Booij <[email protected]>
GitOrigin-RevId: 17244a4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants