这是indexloc提供的服务，不要输入任何密码
Skip to content

Event Triggers: Take webhook url from env ( close #966) #968

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
Merged
merged 13 commits into from
Nov 14, 2018
Merged

Event Triggers: Take webhook url from env ( close #966) #968

merged 13 commits into from
Nov 14, 2018

Conversation

@tirumaraiselvan
Copy link
Contributor

@tirumaraiselvan tirumaraiselvan commented Nov 2, 2018 •
edited
Loading

Description

What component does this PR affect?

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Requires changes from other components? If yes, please mark the components:

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Related Issue

Solution and Design

Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Docs update
  • Community content

Checklist:

  • I have read the contributing guide and my code conforms to the guidelines.
  • This change requires a change in the documentation.
  • I have updated the documentation accordingly.
  • I have added required tests.

@tirumaraiselvan tirumaraiselvan force-pushed the event-triggers-webhook-env branch from 67b6c68 to 3ba752f Compare November 2, 2018 11:14
@tirumaraiselvan tirumaraiselvan force-pushed the event-triggers-webhook-env branch from 3ba752f to a3aca4c Compare November 2, 2018 11:20
@tirumaraiselvan
Copy link
Contributor Author

tirumaraiselvan commented Nov 2, 2018

@dsandip @karthikvt26 Pls add the console changes.

@hasura-bot
Copy link
Contributor

hasura-bot commented Nov 2, 2018

Review app available at: https://hge-ci-pull-968.herokuapp.com

0x777
0x777 reviewed Nov 6, 2018
View reviewed changes
server/src-lib/Hasura/RQL/DDL/Schema/Table.hs Outdated
tDef <- decodeValue tDefVal
addEventTriggerToCache (QualifiedTable sn tn) trid trn tDef (RetryConf nr rint) webhook headers
liftTx $ mkTriggerQ trid trn qt allCols tDef
addEventTriggerToCache (QualifiedTable sn tn) trid trn opsdef retryConf webhookInfo headers
Copy link
Member

@0x777 0x777 Nov 6, 2018

Choose a reason for hiding this comment

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

qt instead of QualifiedTable sn tn

0x777
0x777 reviewed Nov 6, 2018
View reviewed changes
server/src-lib/Hasura/RQL/DDL/Subscribe.hs Outdated
getWebhookInfoFromConf wc = case wc of
WCValue w -> return $ WebhookConfInfo wc w
WCEnv we -> do
mEnv <- liftIO $ lookupEnv (T.unpack we)
Copy link
Member

@0x777 0x777 Nov 6, 2018

Choose a reason for hiding this comment

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

wrap the lookupEnv logic in both the places (getHeader and getWebhook) into a function?

Copy link
Member

@0x777 0x777 Nov 6, 2018

Choose a reason for hiding this comment

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

Also indentation. Why 6 spaces?

Copy link
Contributor Author

@tirumaraiselvan tirumaraiselvan Nov 8, 2018

Choose a reason for hiding this comment

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

The only thing common in these two functions is lookupEnv function and error case. Not sure if it can be improved.

0x777
0x777 reviewed Nov 6, 2018
View reviewed changes
server/src-lib/Hasura/RQL/Types/Subscribe.hs Outdated
parseJSON _ = fail "expecting object for event_trigger_def"

instance ToJSON EventTriggerConf where
toJSON (EventTriggerConf name def (WCValue w) rc headers) = object ["name" .= name
Copy link
Member

@0x777 0x777 Nov 6, 2018

Choose a reason for hiding this comment

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

indentation

@shahidhk shahidhk added the c/server Related to server label Nov 6, 2018
@tirumaraiselvan tirumaraiselvan force-pushed the event-triggers-webhook-env branch from c287ee9 to dac3032 Compare November 8, 2018 07:51
@hasura-bot
Copy link
Contributor

hasura-bot commented Nov 12, 2018

Review app available at: https://hge-ci-pull-968.herokuapp.com

@hasura-bot
Copy link
Contributor

hasura-bot commented Nov 12, 2018

Review app available at: https://hge-ci-pull-968.herokuapp.com

@tirumaraiselvan
Merge branch 'master' into event-triggers-webhook-env
9d82009 
# Conflicts:
#	server/src-lib/Hasura/RQL/Types/Subscribe.hs
praveenweb
praveenweb previously approved these changes Nov 13, 2018
View reviewed changes
Copy link
Member

@praveenweb praveenweb left a comment

Choose a reason for hiding this comment

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

Console changes LGTM

@tirumaraiselvan tirumaraiselvan force-pushed the event-triggers-webhook-env branch 3 times, most recently from a777dca to 8632194 Compare November 13, 2018 08:01
@tirumaraiselvan tirumaraiselvan force-pushed the event-triggers-webhook-env branch from 8632194 to dcab8aa Compare November 13, 2018 08:08
@hasura-bot
Copy link
Contributor

hasura-bot commented Nov 13, 2018

Review app available at: https://hge-ci-pull-968.herokuapp.com

@tirumaraiselvan tirumaraiselvan force-pushed the event-triggers-webhook-env branch from 0640b73 to 5f280a5 Compare November 13, 2018 14:52
@hasura-bot
Copy link
Contributor

hasura-bot commented Nov 13, 2018

Review app available at: https://hge-ci-pull-968.herokuapp.com

@hasura-bot
Copy link
Contributor

hasura-bot commented Nov 14, 2018

Review app available at: https://hge-ci-pull-968.herokuapp.com

@shahidhk
Copy link
Member

shahidhk commented Nov 14, 2018

@rikinsk Can we add the docs for this, since Tiru is on leave?

@0x777 0x777 merged commit 317efb8 into hasura:master Nov 14, 2018
@hasura-bot
Copy link
Contributor

hasura-bot commented Nov 14, 2018

Review app https://hge-ci-pull-968.herokuapp.com is deleted

@dsandip
Copy link
Member

dsandip commented Nov 14, 2018

/heroku deploy

@hasura-bot
Copy link
Contributor

hasura-bot commented Nov 14, 2018

Review app available at: https://hge-ci-pull-968.herokuapp.com

hasura-bot pushed a commit that referenced this pull request Aug 14, 2024
@SamirTalwar @hasura-bot
Add cloud-only crates. (#968)
6fa4d03 
### What

We now support cloud-only crates, which are not open-sourced.

### How

Anything in `crates/cloud` will not be synced with the _graphql-engine_
repository.

In order to facilitate this, we generate and commit a
Cargo.toml/Cargo.lock pair with the cloud-only sections removed. We also
transform the justfile to remove this code.

This includes only a test repository, to ensure that nothing private is
synced.

When this is merged, it should not result in a commit to the
_graphql-engine_ repository.

V3_GIT_ORIGIN_REV_ID: 038839acdf3a97da05bbd4b6278171cc12e7cd71
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@0x777 0x777 0x777 approved these changes

@praveenweb praveenweb praveenweb left review comments

Assignees

No one assigned

Labels

c/server Related to server

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@tirumaraiselvan @hasura-bot @shahidhk @dsandip @0x777 @praveenweb @karthikvt26