From ff2158657d84d26bfafcee3b3515db38e06bd50d Mon Sep 17 00:00:00 2001 From: Tirumarai Selvan A Date: Mon, 28 Jan 2019 19:16:17 +0530 Subject: [PATCH 1/9] remove wreq and set response timeout --- server/src-exec/Main.hs | 5 +- server/src-exec/Ops.hs | 3 +- server/src-lib/Hasura/Events/HTTP.hs | 147 ++++--------------- server/src-lib/Hasura/Events/Lib.hs | 59 +++++--- server/src-lib/Hasura/HTTP.hs | 2 +- server/src-lib/Hasura/RQL/DDL/Subscribe.hs | 8 +- server/src-lib/Hasura/RQL/Types/Subscribe.hs | 16 ++ 7 files changed, 83 insertions(+), 157 deletions(-) diff --git a/server/src-exec/Main.hs b/server/src-exec/Main.hs index c08cf3b1d0818..ab12a4c6a361f 100644 --- a/server/src-exec/Main.hs +++ b/server/src-exec/Main.hs @@ -34,8 +34,6 @@ import Hasura.Server.Query (peelRun) import Hasura.Server.Version (currentVersion) import qualified Database.PG.Query as Q -import qualified Network.HTTP.Client.TLS as TLS -import qualified Network.Wreq.Session as WrqS printErrExit :: forall a . String -> IO a printErrExit = (>> exitFailure) . putStrLn @@ -135,11 +133,10 @@ main = do logEnvHeaders <- getFromEnv False "LOG_HEADERS_FROM_ENV" eventEngineCtx <- atomically $ initEventEngineCtx maxEvThrds evFetchMilliSec - httpSession <- WrqS.newSessionControl Nothing TLS.tlsManagerSettings unLogger logger $ mkGenericStrLog "event_triggers" "starting workers" - void $ C.forkIO $ processEventQueue hloggerCtx logEnvHeaders httpSession pool cacheRef eventEngineCtx + void $ C.forkIO $ processEventQueue hloggerCtx logEnvHeaders httpManager pool cacheRef eventEngineCtx unLogger logger $ mkGenericStrLog "server" "starting API server" diff --git a/server/src-exec/Ops.hs b/server/src-exec/Ops.hs index cfbb3267b3f97..8fe8df9b7b3f2 100644 --- a/server/src-exec/Ops.hs +++ b/server/src-exec/Ops.hs @@ -239,7 +239,6 @@ from4To5 = do migrateMetadataFrom4 = $(unTypeQ (Y.decodeFile "src-rsr/migrate_metadata_from_4_to_5.yaml" :: Q (TExp RQLQuery))) - from3To4 :: (MonadTx m) => m () from3To4 = liftTx $ Q.catchE defaultTxErrorHandler $ do Q.unitQ "ALTER TABLE hdb_catalog.event_triggers ADD COLUMN configuration JSON" () False @@ -257,7 +256,7 @@ from3To4 = liftTx $ Q.catchE defaultTxErrorHandler $ do \, DROP COLUMN headers" () False where uncurryEventTrigger (trn, Q.AltJ tDef, w, nr, rint, Q.AltJ headers) = - EventTriggerConf trn tDef (Just w) Nothing (RetryConf nr rint) headers + EventTriggerConf trn tDef (Just w) Nothing (RetryConf nr rint Nothing) headers updateEventTrigger3To4 etc@(EventTriggerConf name _ _ _ _ _) = Q.unitQ [Q.sql| UPDATE hdb_catalog.event_triggers SET diff --git a/server/src-lib/Hasura/Events/HTTP.hs b/server/src-lib/Hasura/Events/HTTP.hs index 4f75f9f1b7a2c..c6e9eb790a765 100644 --- a/server/src-lib/Hasura/Events/HTTP.hs +++ b/server/src-lib/Hasura/Events/HTTP.hs @@ -1,11 +1,7 @@ module Hasura.Events.HTTP - ( HTTP(..) - , mkAnyHTTPPost - , HTTPErr(..) + ( HTTPErr(..) , HTTPResp(..) , runHTTP - , defaultRetryPolicy - , defaultRetryFn , isNetworkError , isNetworkErrorHC , HLogger @@ -13,38 +9,30 @@ module Hasura.Events.HTTP , ExtraContext(..) ) where -import qualified Control.Retry as R import qualified Data.Aeson as J import qualified Data.Aeson.Casing as J import qualified Data.Aeson.TH as J import qualified Data.ByteString.Lazy as B import qualified Data.CaseInsensitive as CI +import Data.Either import qualified Data.TByteString as TBS import qualified Data.Text as T import qualified Data.Text.Encoding as TE import qualified Data.Text.Encoding.Error as TE import qualified Data.Time.Clock as Time -import qualified Network.HTTP.Client as H -import qualified Network.HTTP.Types as N -import qualified Network.Wreq as W -import qualified Network.Wreq.Session as WS +import qualified Network.HTTP.Client as HTTP +import qualified Network.HTTP.Types as HTTP import qualified System.Log.FastLogger as FL import Control.Exception (try) -import Control.Lens -import Control.Monad.Except (MonadError, throwError) import Control.Monad.IO.Class (MonadIO, liftIO) import Control.Monad.Reader (MonadReader) import Data.Has import Hasura.Logging --- import Data.Monoid import Hasura.Prelude import Hasura.RQL.DDL.Headers import Hasura.RQL.Types.Subscribe --- import Context (HTTPSessionMgr (..)) --- import Log - type HLogger = (LogLevel, EngineLogType, J.Value) -> IO () data ExtraContext @@ -67,12 +55,13 @@ $(J.deriveToJSON (J.aesonDrop 3 J.snakeCase){J.omitNothingFields=True} ''HTTPRes instance ToEngineLog HTTPResp where toEngineLog resp = (LevelInfo, "event-trigger", J.toJSON resp ) -mkHTTPResp :: W.Response B.ByteString -> HTTPResp +mkHTTPResp :: HTTP.Response B.ByteString -> HTTPResp mkHTTPResp resp = HTTPResp - (resp ^. W.responseStatus.W.statusCode) - (map decodeHeader $ resp ^. W.responseHeaders) - (TBS.fromLBS $ resp ^. W.responseBody) + { hrsStatus = HTTP.statusCode $ HTTP.responseStatus resp + , hrsHeaders = map decodeHeader $ HTTP.responseHeaders resp + , hrsBody = TBS.fromLBS $ HTTP.responseBody resp + } where decodeBS = TE.decodeUtf8With TE.lenientDecode decodeHeader (hdrName, hdrVal) @@ -90,8 +79,8 @@ instance ToEngineLog HTTPRespExtra where toEngineLog resp = (LevelInfo, "event-trigger", J.toJSON resp ) data HTTPErr - = HClient !H.HttpException - | HParse !N.Status !String + = HClient !HTTP.HttpException + | HParse !HTTP.Status !String | HStatus !HTTPResp | HOther !String deriving (Show) @@ -101,7 +90,7 @@ instance J.ToJSON HTTPErr where (HClient e) -> ("client", J.toJSON $ show e) (HParse st e) -> ( "parse" - , J.toJSON (N.statusCode st, show e) + , J.toJSON (HTTP.statusCode st, show e) ) (HStatus resp) -> ("status", J.toJSON resp) @@ -114,65 +103,27 @@ instance J.ToJSON HTTPErr where instance ToEngineLog HTTPErr where toEngineLog err = (LevelError, "event-trigger", J.toJSON err ) - - -data HTTP a - = HTTP - { _hMethod :: !String - , _hUrl :: !String - , _hPayload :: !(Maybe J.Value) - , _hFormData :: !(Maybe [W.FormParam]) - -- options modifier - , _hOptions :: W.Options -> W.Options - -- the response parser - , _hParser :: W.Response B.ByteString -> Either HTTPErr a - -- should the operation be retried - , _hRetryFn :: Either HTTPErr a -> Bool - -- the retry policy - , _hRetryPolicy :: R.RetryPolicyM IO - } - --- TODO. Why this istance? --- instance Show (HTTP a) where --- show (HTTP m u p _ _ _ _) = show m ++ " " ++ show u ++ " : " ++ show p - isNetworkError :: HTTPErr -> Bool isNetworkError = \case HClient he -> isNetworkErrorHC he _ -> False -isNetworkErrorHC :: H.HttpException -> Bool +isNetworkErrorHC :: HTTP.HttpException -> Bool isNetworkErrorHC = \case - H.HttpExceptionRequest _ (H.ConnectionFailure _) -> True - H.HttpExceptionRequest _ H.ConnectionTimeout -> True - H.HttpExceptionRequest _ H.ResponseTimeout -> True + HTTP.HttpExceptionRequest _ (HTTP.ConnectionFailure _) -> True + HTTP.HttpExceptionRequest _ HTTP.ConnectionTimeout -> True + HTTP.HttpExceptionRequest _ HTTP.ResponseTimeout -> True _ -> False --- retries on the typical network errors -defaultRetryFn :: Either HTTPErr a -> Bool -defaultRetryFn _ = False - --- full jitter backoff -defaultRetryPolicy :: (MonadIO m) => R.RetryPolicyM m -defaultRetryPolicy = - R.capDelay (120 * 1000 * 1000) (R.fullJitterBackoff (2 * 1000 * 1000)) - <> R.limitRetries 15 - -anyBodyParser :: W.Response B.ByteString -> Either HTTPErr HTTPResp +anyBodyParser :: HTTP.Response B.ByteString -> Either HTTPErr HTTPResp anyBodyParser resp = do let httpResp = mkHTTPResp resp - if respCode >= N.status200 && respCode < N.status300 + if respCode >= HTTP.status200 && respCode < HTTP.status300 then return httpResp else throwError $ HStatus httpResp where - respCode = resp ^. W.responseStatus + respCode = HTTP.responseStatus resp -mkAnyHTTPPost :: String -> Maybe J.Value -> HTTP HTTPResp -mkAnyHTTPPost url payload = - HTTP "POST" url payload Nothing id anyBodyParser - defaultRetryFn defaultRetryPolicy - --- internal logging related types data HTTPReq = HTTPReq { _hrqMethod :: !String @@ -189,63 +140,19 @@ instance ToEngineLog HTTPReq where runHTTP :: ( MonadReader r m - , MonadError HTTPErr m , MonadIO m - , Has WS.Session r , Has HLogger r + , Has HTTP.Manager r ) - => W.Options -> HTTP a -> Maybe ExtraContext -> m a -runHTTP opts http exLog = do - -- try the http request - res <- R.retrying retryPol' retryFn' $ httpWithLogging opts http exLog - - -- process the result - either throwError return res - - where - retryPol' = R.RetryPolicyM $ liftIO . R.getRetryPolicyM (_hRetryPolicy http) - retryFn' _ = return . _hRetryFn http - -httpWithLogging - :: ( MonadReader r m - , MonadIO m - , Has WS.Session r - , Has HLogger r - ) - => W.Options -> HTTP a -> Maybe ExtraContext -> R.RetryStatus -> m (Either HTTPErr a) --- the actual http action -httpWithLogging opts (HTTP method url mPayload mFormParams optsMod bodyParser _ _) exLog retryStatus = do + => HTTP.Request -> Maybe ExtraContext -> m (Either HTTPErr HTTPResp) +runHTTP req exLog = do (logF:: HLogger) <- asks getter - -- log the request - liftIO $ logF $ toEngineLog $ HTTPReq method url mPayload - (R.rsIterNumber retryStatus) (R.rsPreviousDelay retryStatus) - - session <- asks getter - - res <- finallyRunHTTPPlz session + manager <- asks getter + res <- liftIO $ try $ HTTP.httpLbs req manager case res of - Left e -> liftIO $ logF $ toEngineLog $ HClient e - Right resp -> - --liftIO $ print "=======================>" - liftIO $ logF $ toEngineLog $ HTTPRespExtra (mkHTTPResp resp) exLog - --liftIO $ print "<=======================" - - -- return the processed response - return $ either (Left . HClient) bodyParser res - - where - -- set wreq options to ignore status code exceptions - ignoreStatusCodeExceptions _ _ = return () - finalOpts = optsMod opts - & W.checkResponse ?~ ignoreStatusCodeExceptions - - -- the actual function which makes the relevant Wreq calls - finallyRunHTTPPlz sessMgr = - liftIO $ try $ - case (mPayload, mFormParams) of - (Just payload, _) -> WS.customPayloadMethodWith method finalOpts sessMgr url payload - (Nothing, Just fps) -> WS.customPayloadMethodWith method finalOpts sessMgr url fps - (Nothing, Nothing) -> WS.customMethodWith method finalOpts sessMgr url + Left e -> liftIO $ logF $ toEngineLog $ HClient e + Right resp -> liftIO $ logF $ toEngineLog $ HTTPRespExtra (mkHTTPResp resp) exLog + return $ either (Left . HClient) anyBodyParser res mkHLogger :: LoggerCtx -> HLogger mkHLogger (LoggerCtx loggerSet serverLogLevel timeGetter) (logLevel, logTy, logDet) = do diff --git a/server/src-lib/Hasura/Events/Lib.hs b/server/src-lib/Hasura/Events/Lib.hs index 1ab6a7806976a..a97dd001a72ec 100644 --- a/server/src-lib/Hasura/Events/Lib.hs +++ b/server/src-lib/Hasura/Events/Lib.hs @@ -22,10 +22,10 @@ import Hasura.Events.HTTP import Hasura.Prelude import Hasura.RQL.DDL.Headers import Hasura.RQL.Types +import Hasura.Server.Version (currentVersion) import Hasura.SQL.Types import qualified Control.Concurrent.STM.TQueue as TQ -import qualified Control.Lens as CL import qualified Data.ByteString as BS import qualified Data.CaseInsensitive as CI import qualified Data.HashMap.Strict as M @@ -37,9 +37,8 @@ import qualified Data.Text.Encoding.Error as TE import qualified Data.Time.Clock as Time import qualified Database.PG.Query as Q import qualified Hasura.Logging as L -import qualified Network.HTTP.Types as N -import qualified Network.Wreq as W -import qualified Network.Wreq.Session as WS +import qualified Network.HTTP.Client as HTTP +import qualified Network.HTTP.Types as HTTP type Version = T.Text @@ -144,13 +143,13 @@ initEventEngineCtx maxT fetchI = do c <- newTVar 0 return $ EventEngineCtx q c maxT fetchI -processEventQueue :: L.LoggerCtx -> LogEnvHeaders -> WS.Session -> Q.PGPool -> CacheRef -> EventEngineCtx -> IO () -processEventQueue logctx logenv httpSess pool cacheRef eectx = do +processEventQueue :: L.LoggerCtx -> LogEnvHeaders -> HTTP.Manager-> Q.PGPool -> CacheRef -> EventEngineCtx -> IO () +processEventQueue logctx logenv httpMgr pool cacheRef eectx = do threads <- mapM async [fetchThread , consumeThread] void $ waitAny threads where fetchThread = pushEvents (mkHLogger logctx) pool eectx - consumeThread = consumeEvents (mkHLogger logctx) logenv httpSess pool cacheRef eectx + consumeThread = consumeEvents (mkHLogger logctx) logenv httpMgr pool cacheRef eectx pushEvents :: HLogger -> Q.PGPool -> EventEngineCtx -> IO () @@ -163,17 +162,17 @@ pushEvents logger pool eectx = forever $ do threadDelay (fetchI * 1000) consumeEvents - :: HLogger -> LogEnvHeaders -> WS.Session -> Q.PGPool -> CacheRef -> EventEngineCtx -> IO () -consumeEvents logger logenv httpSess pool cacheRef eectx = forever $ do + :: HLogger -> LogEnvHeaders -> HTTP.Manager -> Q.PGPool -> CacheRef -> EventEngineCtx -> IO () +consumeEvents logger logenv httpMgr pool cacheRef eectx = forever $ do event <- atomically $ do let EventEngineCtx q _ _ _ = eectx TQ.readTQueue q - async $ runReaderT (processEvent logenv pool event) (logger, httpSess, cacheRef, eectx) + async $ runReaderT (processEvent logenv pool event) (logger, httpMgr, cacheRef, eectx) processEvent :: ( MonadReader r m , MonadIO m - , Has WS.Session r + , Has HTTP.Manager r , Has HLogger r , Has CacheRef r , Has EventEngineCtx r @@ -217,7 +216,7 @@ processEvent logenv pool e = do cache <- liftIO $ readIORef cacheRef let eti = getEventTriggerInfoFromEvent cache e retryConfM = etiRetryConf <$> eti - retryConf = fromMaybe (RetryConf 0 10) retryConfM + retryConf = fromMaybe defaultRetryConf retryConfM tries = eTries e mretryHeaderSeconds = parseRetryHeader mretryHeader triesExhausted = tries >= rcNumRetries retryConf @@ -247,7 +246,7 @@ processEvent logenv pool e = do tryWebhook :: ( MonadReader r m , MonadIO m - , Has WS.Session r + , Has HTTP.Manager r , Has HLogger r , Has CacheRef r , Has EventEngineCtx r @@ -259,13 +258,17 @@ tryWebhook logenv pool e = do cache <- liftIO $ readIORef cacheRef let meti = getEventTriggerInfoFromEvent cache e case meti of - Nothing -> return $ Left $ HOther "table or event-trigger not found" + Nothing -> return $ Left (HOther "table or event-trigger not found") Just eti -> do let webhook = wciCachedValue $ etiWebhookInfo eti createdAt = eCreatedAt e eventId = eId e headerInfos = etiHeaders eti - headers = map encodeHeader headerInfos + etHeaders = map encodeHeader headerInfos + headers = addDefaultHeaders etHeaders + retryConf = etiRetryConf eti + timeoutSeconds = fromMaybe defaultTimeoutSeconds (rcTimeoutSec retryConf) + responseTimeout = HTTP.responseTimeoutMicro (timeoutSeconds * 1000000) eeCtx <- asks getter -- wait for counter and then increment beforing making http liftIO $ atomically $ do @@ -274,9 +277,16 @@ tryWebhook logenv pool e = do if countThreads >= maxT then retry else modifyTVar' c (+1) - let options = addHeaders headers W.defaults - decodedHeaders = map (decodeHeader headerInfos) $ options CL.^. W.headers - eitherResp <- runExceptT $ runHTTP options (mkAnyHTTPPost (T.unpack webhook) (Just $ toJSON e)) (Just (ExtraContext createdAt eventId)) + -- TODO: Catch exception + initReq <- liftIO $ HTTP.parseRequest $ T.unpack webhook + let req = initReq + { HTTP.method = "POST" + , HTTP.requestHeaders = headers + , HTTP.requestBody = HTTP.RequestBodyLBS (encode e) + , HTTP.responseTimeout = responseTimeout + } + decodedHeaders = map (decodeHeader headerInfos) $ HTTP.requestHeaders req + eitherResp <- runHTTP req (Just (ExtraContext createdAt eventId)) --decrement counter once http is done liftIO $ atomically $ do @@ -314,17 +324,20 @@ tryWebhook logenv pool e = do status (mkWebhookReq (toJSON e) reqHeaders) resp - addHeaders :: [(N.HeaderName, BS.ByteString)] -> W.Options -> W.Options - addHeaders headers opts = foldl (\acc h -> acc CL.& W.header (fst h) CL..~ [snd h] ) opts headers - - encodeHeader :: EventHeaderInfo -> (N.HeaderName, BS.ByteString) + encodeHeader :: EventHeaderInfo -> HTTP.Header encodeHeader (EventHeaderInfo hconf cache) = let (HeaderConf name _) = hconf ciname = CI.mk $ T.encodeUtf8 name value = T.encodeUtf8 cache in (ciname, value) - decodeHeader :: [EventHeaderInfo] -> (N.HeaderName, BS.ByteString) -> HeaderConf + addDefaultHeaders :: [HTTP.Header] -> [HTTP.Header] + addDefaultHeaders hdrs = hdrs ++ + [ (CI.mk "Content-Type", "application/json") + , (CI.mk "User-Agent", "hasura-graphql-engine/" <> T.encodeUtf8 currentVersion ) + ] + + decodeHeader :: [EventHeaderInfo] -> (HTTP.HeaderName, BS.ByteString) -> HeaderConf decodeHeader headerInfos (hdrName, hdrVal) = let name = decodeBS $ CI.original hdrName getName ehi = let (HeaderConf name' _) = ehiHeaderConf ehi diff --git a/server/src-lib/Hasura/HTTP.hs b/server/src-lib/Hasura/HTTP.hs index 0f4358393c56c..70a34c2e6c816 100644 --- a/server/src-lib/Hasura/HTTP.hs +++ b/server/src-lib/Hasura/HTTP.hs @@ -3,7 +3,7 @@ module Hasura.HTTP , HttpException(..) ) where -import Control.Lens hiding ((.=)) +import Control.Lens hiding ((.=)) import Hasura.Prelude import qualified Data.Aeson as J diff --git a/server/src-lib/Hasura/RQL/DDL/Subscribe.hs b/server/src-lib/Hasura/RQL/DDL/Subscribe.hs index b77ae1d6d164c..5ac5bc221c142 100644 --- a/server/src-lib/Hasura/RQL/DDL/Subscribe.hs +++ b/server/src-lib/Hasura/RQL/DDL/Subscribe.hs @@ -32,12 +32,6 @@ import qualified Database.PG.Query as Q data OpVar = OLD | NEW deriving (Show) -defaultNumRetries :: Int -defaultNumRetries = 0 - -defaultRetryInterval :: Int -defaultRetryInterval = 10 - triggerTmplt :: Maybe GingerTmplt triggerTmplt = case parseGingerTmplt $(FE.embedStringFile "src-rsr/trigger.sql.j2") of Left _ -> Nothing @@ -235,7 +229,7 @@ subTableP1 (CreateEventTriggerQuery name qt insert update delete retryConf webho assertCols ti update assertCols ti delete - let rconf = fromMaybe (RetryConf defaultNumRetries defaultRetryInterval) retryConf + let rconf = fromMaybe defaultRetryConf retryConf return (qt, replace, EventTriggerConf name (TriggerOpsDef insert update delete) webhook webhookFromEnv rconf mheaders) where assertCols _ Nothing = return () diff --git a/server/src-lib/Hasura/RQL/Types/Subscribe.hs b/server/src-lib/Hasura/RQL/Types/Subscribe.hs index 34d99e4f203eb..eb34cb497ced4 100644 --- a/server/src-lib/Hasura/RQL/Types/Subscribe.hs +++ b/server/src-lib/Hasura/RQL/Types/Subscribe.hs @@ -17,6 +17,9 @@ module Hasura.RQL.Types.Subscribe , EventHeaderInfo(..) , WebhookConf(..) , WebhookConfInfo(..) + + , defaultRetryConf + , defaultTimeoutSeconds ) where import Data.Aeson @@ -58,10 +61,23 @@ data SubscribeOpSpec $(deriveJSON (aesonDrop 3 snakeCase){omitNothingFields=True} ''SubscribeOpSpec) +defaultNumRetries :: Int +defaultNumRetries = 0 + +defaultRetryInterval :: Int +defaultRetryInterval = 10 + +defaultTimeoutSeconds:: Int +defaultTimeoutSeconds = 60 + +defaultRetryConf :: RetryConf +defaultRetryConf = RetryConf defaultNumRetries defaultRetryInterval (Just defaultTimeoutSeconds) + data RetryConf = RetryConf { rcNumRetries :: !Int , rcIntervalSec :: !Int + , rcTimeoutSec :: !(Maybe Int) } deriving (Show, Eq, Lift) $(deriveJSON (aesonDrop 2 snakeCase){omitNothingFields=True} ''RetryConf) From 1c2d6ef216bfb7eb7f19cc0f8c5927d972a915c4 Mon Sep 17 00:00:00 2001 From: Tirumarai Selvan A Date: Mon, 28 Jan 2019 21:24:21 +0530 Subject: [PATCH 2/9] parseRequest in try block --- server/src-lib/Hasura/Events/Lib.hs | 84 +++++++++++++++-------------- 1 file changed, 44 insertions(+), 40 deletions(-) diff --git a/server/src-lib/Hasura/Events/Lib.hs b/server/src-lib/Hasura/Events/Lib.hs index a97dd001a72ec..8f35ad2248a8a 100644 --- a/server/src-lib/Hasura/Events/Lib.hs +++ b/server/src-lib/Hasura/Events/Lib.hs @@ -10,6 +10,7 @@ module Hasura.Events.Lib import Control.Concurrent (threadDelay) import Control.Concurrent.Async (async, waitAny) import Control.Concurrent.STM.TVar +import Control.Exception (try) import Control.Monad.STM (STM, atomically, retry) import Data.Aeson import Data.Aeson.Casing @@ -103,14 +104,14 @@ data WebhookResponse } $(deriveToJSON (aesonDrop 4 snakeCase){omitNothingFields=True} ''WebhookResponse) -data InitError = InitError { _ieMessage :: TBS.TByteString} -$(deriveToJSON (aesonDrop 3 snakeCase){omitNothingFields=True} ''InitError) +data ClientError = ClientError { _ceMessage :: TBS.TByteString} +$(deriveToJSON (aesonDrop 3 snakeCase){omitNothingFields=True} ''ClientError) -data Response = ResponseType1 WebhookResponse | ResponseType2 InitError +data Response = ResponseType1 WebhookResponse | ResponseType2 ClientError instance ToJSON Response where toJSON (ResponseType1 resp) = object ["type" .= String "webhook_response", "data" .= toJSON resp, "version" .= invocationVersion] - toJSON (ResponseType2 err) = object ["type" .= String "init_error", "data" .= toJSON err, "version" .= invocationVersion] + toJSON (ResponseType2 err) = object ["type" .= String "client_error", "data" .= toJSON err, "version" .= invocationVersion] data Invocation = Invocation @@ -278,46 +279,49 @@ tryWebhook logenv pool e = do then retry else modifyTVar' c (+1) -- TODO: Catch exception - initReq <- liftIO $ HTTP.parseRequest $ T.unpack webhook - let req = initReq + initReqE <- liftIO $ try $ HTTP.parseRequest $ T.unpack webhook + case initReqE of + Left excp -> return $ Left (HClient excp) + Right initReq -> do + let req = initReq { HTTP.method = "POST" , HTTP.requestHeaders = headers , HTTP.requestBody = HTTP.RequestBodyLBS (encode e) , HTTP.responseTimeout = responseTimeout } - decodedHeaders = map (decodeHeader headerInfos) $ HTTP.requestHeaders req - eitherResp <- runHTTP req (Just (ExtraContext createdAt eventId)) + decodedHeaders = map (decodeHeader headerInfos) $ HTTP.requestHeaders req + eitherResp <- runHTTP req (Just (ExtraContext createdAt eventId)) --decrement counter once http is done - liftIO $ atomically $ do - let EventEngineCtx _ c _ _ = eeCtx - modifyTVar' c (\v -> v - 1) - - finally <- liftIO $ runExceptT $ case eitherResp of - Left err -> - case err of - HClient excp -> let errMsg = TBS.fromLBS $ encode $ show excp - in runFailureQ pool $ mkInvo e 1000 decodedHeaders errMsg [] - HParse _ detail -> let errMsg = TBS.fromLBS $ encode detail - in runFailureQ pool $ mkInvo e 1001 decodedHeaders errMsg [] - HStatus errResp -> let respPayload = hrsBody errResp - respHeaders = hrsHeaders errResp - respStatus = hrsStatus errResp - in runFailureQ pool $ mkInvo e respStatus decodedHeaders respPayload respHeaders - HOther detail -> let errMsg = (TBS.fromLBS $ encode detail) - in runFailureQ pool $ mkInvo e 500 decodedHeaders errMsg [] - Right resp -> let respPayload = hrsBody resp - respHeaders = hrsHeaders resp - respStatus = hrsStatus resp - in runSuccessQ pool e $ mkInvo e respStatus decodedHeaders respPayload respHeaders - case finally of - Left err -> liftIO $ logger $ L.toEngineLog $ EventInternalErr err - Right _ -> return () - return eitherResp + liftIO $ atomically $ do + let EventEngineCtx _ c _ _ = eeCtx + modifyTVar' c (\v -> v - 1) + + finally <- liftIO $ runExceptT $ case eitherResp of + Left err -> + case err of + HClient excp -> let errMsg = TBS.fromLBS $ encode $ show excp + in runFailureQ pool $ mkInvo e 1000 decodedHeaders errMsg [] + HParse _ detail -> let errMsg = TBS.fromLBS $ encode detail + in runFailureQ pool $ mkInvo e 1001 decodedHeaders errMsg [] + HStatus errResp -> let respPayload = hrsBody errResp + respHeaders = hrsHeaders errResp + respStatus = hrsStatus errResp + in runFailureQ pool $ mkInvo e respStatus decodedHeaders respPayload respHeaders + HOther detail -> let errMsg = (TBS.fromLBS $ encode detail) + in runFailureQ pool $ mkInvo e 500 decodedHeaders errMsg [] + Right resp -> let respPayload = hrsBody resp + respHeaders = hrsHeaders resp + respStatus = hrsStatus resp + in runSuccessQ pool e $ mkInvo e respStatus decodedHeaders respPayload respHeaders + case finally of + Left err -> liftIO $ logger $ L.toEngineLog $ EventInternalErr err + Right _ -> return () + return eitherResp where mkInvo :: Event -> Int -> [HeaderConf] -> TBS.TByteString -> [HeaderConf] -> Invocation mkInvo e' status reqHeaders respBody respHeaders - = let resp = if isInitError status then mkErr respBody else mkResp status respBody respHeaders + = let resp = if isClientError status then mkClientErr respBody else mkResp status respBody respHeaders in Invocation (eId e') @@ -359,17 +363,17 @@ tryWebhook logenv pool e = do let wr = WebhookResponse payload (mkMaybe headers) status in ResponseType1 wr - mkErr :: TBS.TByteString -> Response - mkErr message = - let ir = InitError message - in ResponseType2 ir + mkClientErr :: TBS.TByteString -> Response + mkClientErr message = + let cerr = ClientError message + in ResponseType2 cerr mkMaybe :: [a] -> Maybe [a] mkMaybe [] = Nothing mkMaybe x = Just x - isInitError :: Int -> Bool - isInitError status = status >= 1000 + isClientError :: Int -> Bool + isClientError status = status >= 1000 getEventTriggerInfoFromEvent :: SchemaCache -> Event -> Maybe EventTriggerInfo From bea41034d0570326dd7132778370ed7c9fbb1cca Mon Sep 17 00:00:00 2001 From: Tirumarai Selvan A Date: Mon, 28 Jan 2019 21:24:21 +0530 Subject: [PATCH 3/9] parseRequest in try block --- server/src-lib/Hasura/Events/Lib.hs | 86 +++++++++++++++-------------- 1 file changed, 45 insertions(+), 41 deletions(-) diff --git a/server/src-lib/Hasura/Events/Lib.hs b/server/src-lib/Hasura/Events/Lib.hs index a97dd001a72ec..09aebb671c638 100644 --- a/server/src-lib/Hasura/Events/Lib.hs +++ b/server/src-lib/Hasura/Events/Lib.hs @@ -10,6 +10,7 @@ module Hasura.Events.Lib import Control.Concurrent (threadDelay) import Control.Concurrent.Async (async, waitAny) import Control.Concurrent.STM.TVar +import Control.Exception (try) import Control.Monad.STM (STM, atomically, retry) import Data.Aeson import Data.Aeson.Casing @@ -103,14 +104,14 @@ data WebhookResponse } $(deriveToJSON (aesonDrop 4 snakeCase){omitNothingFields=True} ''WebhookResponse) -data InitError = InitError { _ieMessage :: TBS.TByteString} -$(deriveToJSON (aesonDrop 3 snakeCase){omitNothingFields=True} ''InitError) +data ClientError = ClientError { _ceMessage :: TBS.TByteString} +$(deriveToJSON (aesonDrop 3 snakeCase){omitNothingFields=True} ''ClientError) -data Response = ResponseType1 WebhookResponse | ResponseType2 InitError +data Response = ResponseType1 WebhookResponse | ResponseType2 ClientError instance ToJSON Response where toJSON (ResponseType1 resp) = object ["type" .= String "webhook_response", "data" .= toJSON resp, "version" .= invocationVersion] - toJSON (ResponseType2 err) = object ["type" .= String "init_error", "data" .= toJSON err, "version" .= invocationVersion] + toJSON (ResponseType2 err) = object ["type" .= String "client_error", "data" .= toJSON err, "version" .= invocationVersion] data Invocation = Invocation @@ -277,47 +278,50 @@ tryWebhook logenv pool e = do if countThreads >= maxT then retry else modifyTVar' c (+1) - -- TODO: Catch exception - initReq <- liftIO $ HTTP.parseRequest $ T.unpack webhook - let req = initReq + + initReqE <- liftIO $ try $ HTTP.parseRequest $ T.unpack webhook + case initReqE of + Left excp -> return $ Left (HClient excp) + Right initReq -> do + let req = initReq { HTTP.method = "POST" , HTTP.requestHeaders = headers , HTTP.requestBody = HTTP.RequestBodyLBS (encode e) , HTTP.responseTimeout = responseTimeout } - decodedHeaders = map (decodeHeader headerInfos) $ HTTP.requestHeaders req - eitherResp <- runHTTP req (Just (ExtraContext createdAt eventId)) + decodedHeaders = map (decodeHeader headerInfos) $ HTTP.requestHeaders req + eitherResp <- runHTTP req (Just (ExtraContext createdAt eventId)) --decrement counter once http is done - liftIO $ atomically $ do - let EventEngineCtx _ c _ _ = eeCtx - modifyTVar' c (\v -> v - 1) - - finally <- liftIO $ runExceptT $ case eitherResp of - Left err -> - case err of - HClient excp -> let errMsg = TBS.fromLBS $ encode $ show excp - in runFailureQ pool $ mkInvo e 1000 decodedHeaders errMsg [] - HParse _ detail -> let errMsg = TBS.fromLBS $ encode detail - in runFailureQ pool $ mkInvo e 1001 decodedHeaders errMsg [] - HStatus errResp -> let respPayload = hrsBody errResp - respHeaders = hrsHeaders errResp - respStatus = hrsStatus errResp - in runFailureQ pool $ mkInvo e respStatus decodedHeaders respPayload respHeaders - HOther detail -> let errMsg = (TBS.fromLBS $ encode detail) - in runFailureQ pool $ mkInvo e 500 decodedHeaders errMsg [] - Right resp -> let respPayload = hrsBody resp - respHeaders = hrsHeaders resp - respStatus = hrsStatus resp - in runSuccessQ pool e $ mkInvo e respStatus decodedHeaders respPayload respHeaders - case finally of - Left err -> liftIO $ logger $ L.toEngineLog $ EventInternalErr err - Right _ -> return () - return eitherResp + liftIO $ atomically $ do + let EventEngineCtx _ c _ _ = eeCtx + modifyTVar' c (\v -> v - 1) + + finally <- liftIO $ runExceptT $ case eitherResp of + Left err -> + case err of + HClient excp -> let errMsg = TBS.fromLBS $ encode $ show excp + in runFailureQ pool $ mkInvo e 1000 decodedHeaders errMsg [] + HParse _ detail -> let errMsg = TBS.fromLBS $ encode detail + in runFailureQ pool $ mkInvo e 1001 decodedHeaders errMsg [] + HStatus errResp -> let respPayload = hrsBody errResp + respHeaders = hrsHeaders errResp + respStatus = hrsStatus errResp + in runFailureQ pool $ mkInvo e respStatus decodedHeaders respPayload respHeaders + HOther detail -> let errMsg = (TBS.fromLBS $ encode detail) + in runFailureQ pool $ mkInvo e 500 decodedHeaders errMsg [] + Right resp -> let respPayload = hrsBody resp + respHeaders = hrsHeaders resp + respStatus = hrsStatus resp + in runSuccessQ pool e $ mkInvo e respStatus decodedHeaders respPayload respHeaders + case finally of + Left err -> liftIO $ logger $ L.toEngineLog $ EventInternalErr err + Right _ -> return () + return eitherResp where mkInvo :: Event -> Int -> [HeaderConf] -> TBS.TByteString -> [HeaderConf] -> Invocation mkInvo e' status reqHeaders respBody respHeaders - = let resp = if isInitError status then mkErr respBody else mkResp status respBody respHeaders + = let resp = if isClientError status then mkClientErr respBody else mkResp status respBody respHeaders in Invocation (eId e') @@ -359,17 +363,17 @@ tryWebhook logenv pool e = do let wr = WebhookResponse payload (mkMaybe headers) status in ResponseType1 wr - mkErr :: TBS.TByteString -> Response - mkErr message = - let ir = InitError message - in ResponseType2 ir + mkClientErr :: TBS.TByteString -> Response + mkClientErr message = + let cerr = ClientError message + in ResponseType2 cerr mkMaybe :: [a] -> Maybe [a] mkMaybe [] = Nothing mkMaybe x = Just x - isInitError :: Int -> Bool - isInitError status = status >= 1000 + isClientError :: Int -> Bool + isClientError status = status >= 1000 getEventTriggerInfoFromEvent :: SchemaCache -> Event -> Maybe EventTriggerInfo From 942a0b12068af157c2970f24eff1b854dc806746 Mon Sep 17 00:00:00 2001 From: wawhal Date: Tue, 29 Jan 2019 15:09:07 +0530 Subject: [PATCH 4/9] add trigger retry timeout functionality to console --- .../Services/EventTrigger/Add/AddActions.js | 11 ++ .../Services/EventTrigger/Add/AddState.js | 2 +- .../Services/EventTrigger/Add/AddTrigger.js | 136 ++++++++++++------ .../Services/EventTrigger/Modify/Actions.js | 11 ++ .../Services/EventTrigger/Modify/Modify.js | 1 + .../EventTrigger/Modify/RetryConfEditor.js | 68 ++++++++- .../Services/EventTrigger/Modify/State.js | 1 + .../EventTrigger/TableCommon/Table.scss | 14 +- console/src/helpers/semver.js | 1 + 9 files changed, 191 insertions(+), 54 deletions(-) diff --git a/console/src/components/Services/EventTrigger/Add/AddActions.js b/console/src/components/Services/EventTrigger/Add/AddActions.js index 3e50a738e5d84..58ceb38f7ed4a 100644 --- a/console/src/components/Services/EventTrigger/Add/AddActions.js +++ b/console/src/components/Services/EventTrigger/Add/AddActions.js @@ -19,6 +19,7 @@ const SET_SCHEMANAME = 'AddTrigger/SET_SCHEMANAME'; const SET_WEBHOOK_URL = 'AddTrigger/SET_WEBHOOK_URL'; const SET_RETRY_NUM = 'AddTrigger/SET_RETRY_NUM'; const SET_RETRY_INTERVAL = 'AddTrigger/SET_RETRY_INTERVAL'; +const SET_RETRY_TIMEOUT = 'AddTrigger/SET_RETRY_TIMEOUT'; const MAKING_REQUEST = 'AddTrigger/MAKING_REQUEST'; const REQUEST_SUCCESS = 'AddTrigger/REQUEST_SUCCESS'; const REQUEST_ERROR = 'AddTrigger/REQUEST_ERROR'; @@ -40,6 +41,7 @@ const setSchemaName = value => ({ type: SET_SCHEMANAME, value }); const setWebhookURL = value => ({ type: SET_WEBHOOK_URL, value }); const setRetryNum = value => ({ type: SET_RETRY_NUM, value }); const setRetryInterval = value => ({ type: SET_RETRY_INTERVAL, value }); +const setRetryTimeout = value => ({ type: SET_RETRY_TIMEOUT, value }); const setDefaults = () => ({ type: SET_DEFAULTS }); const addHeader = () => ({ type: ADD_HEADER }); const removeHeader = i => ({ type: REMOVE_HEADER, index: i }); @@ -341,6 +343,14 @@ const addTriggerReducer = (state = defaultState, action) => { interval_sec: parseInt(action.value, 10), }, }; + case SET_RETRY_TIMEOUT: + return { + ...state, + retryConf: { + ...state.retryConf, + timeout_sec: parseInt(action.value, 10), + }, + }; case SET_TABLENAME: return { ...state, tableName: action.value }; case SET_SCHEMANAME: @@ -382,6 +392,7 @@ export { setWebhookURL, setRetryNum, setRetryInterval, + setRetryTimeout, createTrigger, fetchTableListBySchema, operationToggleColumn, diff --git a/console/src/components/Services/EventTrigger/Add/AddState.js b/console/src/components/Services/EventTrigger/Add/AddState.js index 10084ff040dda..dbc13699ff2a5 100644 --- a/console/src/components/Services/EventTrigger/Add/AddState.js +++ b/console/src/components/Services/EventTrigger/Add/AddState.js @@ -7,7 +7,7 @@ const defaultState = { selectedOperations: { insert: false, update: false, delete: false }, webhookURL: '', webhookUrlType: 'url', - retryConf: null, + retryConf: {}, ongoingRequest: false, lastError: null, internalError: null, diff --git a/console/src/components/Services/EventTrigger/Add/AddTrigger.js b/console/src/components/Services/EventTrigger/Add/AddTrigger.js index 86d493638bba7..ea33cdc7effe5 100644 --- a/console/src/components/Services/EventTrigger/Add/AddTrigger.js +++ b/console/src/components/Services/EventTrigger/Add/AddTrigger.js @@ -17,6 +17,7 @@ import { setWebhookURL, setRetryNum, setRetryInterval, + setRetryTimeout, operationToggleColumn, operationToggleAllColumns, setOperationSelection, @@ -40,6 +41,7 @@ class AddTrigger extends Component { advancedExpanded: false, supportColumnChangeFeature: false, supportWebhookEnv: false, + supportRetryTimeout: false, }; } componentDidMount() { @@ -48,6 +50,7 @@ class AddTrigger extends Component { if (this.props.serverVersion) { this.checkSemVer(this.props.serverVersion).then(() => { this.checkWebhookEnvSupport(this.props.serverVersion); + this.checkRetryTimeoutSupport(this.props.serverVersion); }); } } @@ -55,6 +58,7 @@ class AddTrigger extends Component { if (nextProps.serverVersion !== this.props.serverVersion) { this.checkSemVer(nextProps.serverVersion).then(() => { this.checkWebhookEnvSupport(nextProps.serverVersion); + this.checkRetryTimeoutSupport(this.props.serverVersion); }); } } @@ -83,7 +87,13 @@ class AddTrigger extends Component { checkWebhookEnvSupport(version) { const supportWebhookEnv = semverCheck('webhookEnvSupport', version); - this.setState({ ...this.state, supportWebhookEnv }); + this.setState({ supportWebhookEnv }); + return Promise.resolve(); + } + + checkRetryTimeoutSupport(version) { + const supportRetryTimeout = semverCheck('triggerRetryTimeout', version); + this.setState({ supportRetryTimeout }); return Promise.resolve(); } @@ -131,6 +141,14 @@ class AddTrigger extends Component { errorMsg = 'Retry interval is not valid'; customMsg = 'Retry interval cannot be empty and can only be numbers'; } + if ( + this.state.supportRetryTimeout && + isNaN(parseInt(this.props.retryConf.timeout_sec, 10)) + ) { + isValid = false; + errorMsg = 'Timeout is not valid'; + customMsg = 'Timeout cannot be empty and can only be numbers'; + } } else if (this.props.selectedOperations.insert) { // check if columns are selected. if (this.props.operations.insert.length === 0) { @@ -200,7 +218,7 @@ class AddTrigger extends Component { webhookUrlType, } = this.props; - const { supportColumnChangeFeature } = this.state; + const { supportColumnChangeFeature, supportRetryTimeout } = this.state; const styles = require('../TableCommon/Table.scss'); let createBtnText = 'Create'; @@ -687,50 +705,80 @@ class AddTrigger extends Component { } >

Retry Logic

-
- - { - dispatch(setRetryNum(e.target.value)); - }} - data-test="no-of-retries" - className={styles.display_inline + ' form-control'} - type="text" - placeholder="no of retries" - /> +
+
+ +
+
+ { + dispatch(setRetryNum(e.target.value)); + }} + data-test="no-of-retries" + className={`${styles.display_inline} form-control ${ + styles.width300 + }`} + type="text" + placeholder="no of retries" + /> +
-
- - { - dispatch(setRetryInterval(e.target.value)); - }} - data-test="interval-seconds" - className={styles.display_inline + ' form-control'} - type="text" - placeholder="interval time in seconds" - /> +
+
+ +
+
+ { + dispatch(setRetryInterval(e.target.value)); + }} + data-test="interval-seconds" + className={`${styles.display_inline} form-control ${ + styles.width300 + }`} + type="text" + placeholder="interval time in seconds" + /> +
+ {supportRetryTimeout && ( +
+
+ +
+
+ { + dispatch(setRetryTimeout(e.target.value)); + }} + data-test="interval-seconds" + className={`${styles.display_inline} form-control ${ + styles.width300 + }`} + type="text" + placeholder="timeout in seconds" + /> +
+
+ )}
({ type: SET_WEBHOOK_URL_TYPE, data }); const SET_RETRY_NUM = 'ModifyTrigger/SET_RETRY_NUM'; const SET_RETRY_INTERVAL = 'ModifyTrigger/SET_RETRY_INTERVAL'; +const SET_RETRY_TIMEOUT = 'ModifyTrigger/SET_RETRY_TIMEOUT'; export const setRetryNum = data => ({ type: SET_RETRY_NUM, data }); export const setRetryInterval = data => ({ type: SET_RETRY_INTERVAL, data }); +export const setRetryTimeout = data => ({ type: SET_RETRY_TIMEOUT, data }); const TOGGLE_COLUMN = 'ModifyTrigger/TOGGLE_COLUMNS'; const TOGGLE_QUERY_TYPE = 'ModifyTrigger/TOGGLE_QUERY_TYPE_SELECTED'; @@ -104,6 +106,7 @@ export const save = (property, triggerName) => { upPayload.retry_conf = { num_retries: modifyTrigger.retryConf.numRetrys, interval_sec: modifyTrigger.retryConf.retryInterval, + timeout_sec: modifyTrigger.retryConf.timeout, }; } else if (property === 'headers') { delete upPayload.headers; @@ -238,6 +241,14 @@ const reducer = (state = defaultState, action) => { retryInterval: action.data, }, }; + case SET_RETRY_TIMEOUT: + return { + ...state, + retryConf: { + ...state.retryConf, + timeout: action.data, + }, + }; case ADD_HEADER: return { ...state, diff --git a/console/src/components/Services/EventTrigger/Modify/Modify.js b/console/src/components/Services/EventTrigger/Modify/Modify.js index 4352fd82e5f2b..1ca7700f2edcb 100644 --- a/console/src/components/Services/EventTrigger/Modify/Modify.js +++ b/console/src/components/Services/EventTrigger/Modify/Modify.js @@ -91,6 +91,7 @@ class Modify extends React.Component { modifyTrigger={modifyTrigger} styles={styles} save={() => dispatch(save('retry', modifyTriggerName))} + serverVersion={this.props.serverVersion} dispatch={dispatch} /> { const { dispatch } = this.props; const retryConf = this.props.retryConf || {}; dispatch(setRetryNum(retryConf.num_retries || 0)); dispatch(setRetryInterval(retryConf.interval_sec || 10)); + if (this.state.supportRetryTimeout) { + dispatch(setRetryTimeout(retryConf.timeout_sec || 60)); + } }; + checkRetryTimeoutSupport(version) { + const supportRetryTimeout = semverCheck('triggerRetryTimeout', version); + this.setState({ supportRetryTimeout }); + } + validateAndSave = () => { const { dispatch, modifyTrigger: { - retryConf: { numRetrys, retryInterval }, + retryConf: { numRetrys, retryInterval, timeout }, }, } = this.props; if (isNaN(numRetrys)) { @@ -31,12 +57,21 @@ class RetryConfEditor extends React.Component { } dispatch(setRetryInterval(parseInt(retryInterval, 10))); + if (this.state.supportRetryTimeout) { + if (isNaN(retryInterval)) { + alert('Retry interval should be an integer!'); + return; + } + dispatch(setRetryTimeout(parseInt(timeout, 10))); + } + this.props.save(); }; render() { const { styles, dispatch, modifyTrigger } = this.props; const retryConf = this.props.retryConf || {}; + const { supportRetryTimeout } = this.state; const collapsed = toggleButton => (
{toggleButton('Edit')} @@ -46,7 +81,7 @@ class RetryConfEditor extends React.Component { Number of retries:
- {retryConf.num_retries || 0} + {retryConf.num_retries}
@@ -54,9 +89,17 @@ class RetryConfEditor extends React.Component { Retry Interval (sec):
- {retryConf.interval_sec || 0} + {retryConf.interval_sec}
+ {supportRetryTimeout && ( +
+
Timeout (sec):
+
+ {retryConf.timeout_sec} +
+
+ )} ); @@ -95,6 +138,23 @@ class RetryConfEditor extends React.Component { /> + {this.state.supportRetryTimeout && ( +
+
+ Timeout (sec):  +
+
+ dispatch(setRetryTimeout(e.target.value))} + /> +
+
+ )} {saveButton(this.validateAndSave)} diff --git a/console/src/components/Services/EventTrigger/Modify/State.js b/console/src/components/Services/EventTrigger/Modify/State.js index ec4c317cdde45..433a9607d9f5a 100644 --- a/console/src/components/Services/EventTrigger/Modify/State.js +++ b/console/src/components/Services/EventTrigger/Modify/State.js @@ -5,6 +5,7 @@ const defaultState = { retryConf: { numRetrys: 0, retryInterval: 10, + timeout: 60, }, ongoingRequest: false, lastError: null, diff --git a/console/src/components/Services/EventTrigger/TableCommon/Table.scss b/console/src/components/Services/EventTrigger/TableCommon/Table.scss index abdfd9e62b386..c4d9401b0c11a 100644 --- a/console/src/components/Services/EventTrigger/TableCommon/Table.scss +++ b/console/src/components/Services/EventTrigger/TableCommon/Table.scss @@ -213,19 +213,19 @@ a.expanded { margin-bottom: 10px; } margin-bottom: 10px; - label { - cursor: pointer; - } } .retryLabel { display: block; - margin-bottom: 10px !important; } .retrySection { - width: 300px; margin-right: 20px; + width: 100%; + float: left; + margin-bottom: 10px; + display: flex; + align-items: center; } .invocationsSection { @@ -349,4 +349,8 @@ a.expanded { i:hover { color: #4D4D4D ; } +} + +.width300 { + width: 300px; } \ No newline at end of file diff --git a/console/src/helpers/semver.js b/console/src/helpers/semver.js index a89af61509cad..33e99c99ebfad 100644 --- a/console/src/helpers/semver.js +++ b/console/src/helpers/semver.js @@ -14,6 +14,7 @@ const componentsSemver = { insertPermRestrictColumns: '1.0.0-alpha28', permHideUpsertSection: '1.0.0-alpha32', customFunctionSection: '1.0.0-alpha36', + triggerRetryTimeout: '1.0.0-alpha37', }; const getPreRelease = version => { From 8eb322642e97c1b574c1283e5532207a49228035 Mon Sep 17 00:00:00 2001 From: wawhal Date: Tue, 29 Jan 2019 16:23:56 +0530 Subject: [PATCH 5/9] add tests --- console/cypress/helpers/eventHelpers.js | 1 + .../integration/events/create-trigger/spec.js | 4 ++- .../integration/validators/validators.js | 32 +++++++++++++++++-- .../Services/EventTrigger/Add/AddTrigger.js | 2 +- 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/console/cypress/helpers/eventHelpers.js b/console/cypress/helpers/eventHelpers.js index ebeaa84d3df66..c13b640899828 100644 --- a/console/cypress/helpers/eventHelpers.js +++ b/console/cypress/helpers/eventHelpers.js @@ -7,6 +7,7 @@ export const getTableName = (i, testName = '') => export const getWebhookURL = () => 'http://httpbin.org/post'; export const getNoOfRetries = () => '5'; export const getIntervalSeconds = () => '10'; +export const getTimeoutSeconds = () => '25'; export const getElementFromAlias = alias => `[data-test=${alias}]`; export const makeDataAPIUrl = dataApiUrl => `${dataApiUrl}/v1/query`; export const makeDataAPIOptions = (dataApiUrl, key, body) => ({ diff --git a/console/cypress/integration/events/create-trigger/spec.js b/console/cypress/integration/events/create-trigger/spec.js index 1dd931f0b41d1..fab25b8d8dc0c 100644 --- a/console/cypress/integration/events/create-trigger/spec.js +++ b/console/cypress/integration/events/create-trigger/spec.js @@ -5,6 +5,7 @@ import { getWebhookURL, getNoOfRetries, getIntervalSeconds, + getTimeoutSeconds, baseUrl, } from '../../../helpers/eventHelpers'; import { getColName } from '../../../helpers/dataHelpers'; @@ -91,6 +92,7 @@ export const passCT = () => { // retry configuration cy.get(getElementFromAlias('no-of-retries')).type(getNoOfRetries()); cy.get(getElementFromAlias('interval-seconds')).type(getIntervalSeconds()); + cy.get(getElementFromAlias('timeout-seconds')).type(getTimeoutSeconds()); // Click on create cy.get(getElementFromAlias('trigger-create')).click(); @@ -163,7 +165,7 @@ export const deleteCTTestTrigger = () => { // Match the URL cy.url().should('eq', `${baseUrl}/events/manage/triggers`); // Validate - validateCTrigger(getTriggerName(0, testName), 'success'); + validateCTrigger(getTriggerName(0, testName), 'failure'); }; export const deleteCTTestTable = () => { diff --git a/console/cypress/integration/validators/validators.js b/console/cypress/integration/validators/validators.js index c06217ccd0fe7..4d827219d8a36 100644 --- a/console/cypress/integration/validators/validators.js +++ b/console/cypress/integration/validators/validators.js @@ -1,6 +1,12 @@ import { makeDataAPIOptions, getColName } from '../../helpers/dataHelpers'; import { migrateModeUrl } from '../../helpers/common'; import { toggleOnMigrationMode } from '../data/migration-mode/utils'; +import { + getNoOfRetries, + getIntervalSeconds, + getTimeoutSeconds, +} from '../../helpers/eventHelpers'; + // ***************** UTIL FUNCTIONS ************************** let accessKey; @@ -339,7 +345,7 @@ export const validateCTrigger = (triggerName, result) => { type: 'select', args: { table: { name: 'event_triggers', schema: 'hdb_catalog' }, - columns: ['table_name'], + columns: ['*', 'table_name'], where: { name: triggerName }, }, }; @@ -347,8 +353,30 @@ export const validateCTrigger = (triggerName, result) => { cy.request(requestOptions).then(response => { if (result === 'success') { expect(response.status === 200).to.be.true; + expect(response.body.length === 1).to.be.true; + const trigger = response.body[0]; + expect(trigger.configuration.definition.insert.columns.length === 3).to.be + .true; + expect(trigger.configuration.definition.update.columns.length === 3).to.be + .true; + expect(trigger.configuration.definition.delete.columns.length === 3).to.be + .true; + expect( + trigger.configuration.retry_conf.interval_sec === + parseInt(getIntervalSeconds(), 10) + ).to.be.true; + expect( + trigger.configuration.retry_conf.num_retries === + parseInt(getNoOfRetries(), 10) + ).to.be.true; + expect( + trigger.configuration.retry_conf.timeout_sec === + parseInt(getTimeoutSeconds(), 10) + ).to.be.true; + expect(trigger.schema_name === 'public').to.be.true; + expect(trigger.table_name === 'apic_test_table_ctr_0').to.be.true; } else { - expect(response.status === 200).to.be.false; + expect(response.body.length === 0).to.be.true; } }); }; diff --git a/console/src/components/Services/EventTrigger/Add/AddTrigger.js b/console/src/components/Services/EventTrigger/Add/AddTrigger.js index ea33cdc7effe5..7b10e8b9ff576 100644 --- a/console/src/components/Services/EventTrigger/Add/AddTrigger.js +++ b/console/src/components/Services/EventTrigger/Add/AddTrigger.js @@ -769,7 +769,7 @@ class AddTrigger extends Component { onChange={e => { dispatch(setRetryTimeout(e.target.value)); }} - data-test="interval-seconds" + data-test="timeout-seconds" className={`${styles.display_inline} form-control ${ styles.width300 }`} From b836919d416ac4fdc687462ec91acbd7a41e3804 Mon Sep 17 00:00:00 2001 From: Tirumarai Selvan A Date: Wed, 30 Jan 2019 17:13:22 +0530 Subject: [PATCH 6/9] add tests for timeout --- server/tests-py/context.py | 16 +++++- .../event_triggers/retry_conf/setup.yaml | 50 +++++++++++++++++++ .../event_triggers/retry_conf/teardown.yaml | 4 +- server/tests-py/test_events.py | 26 ++++++++++ 4 files changed, 94 insertions(+), 2 deletions(-) diff --git a/server/tests-py/context.py b/server/tests-py/context.py index f91f364091202..0e6d3e5ff9bfb 100644 --- a/server/tests-py/context.py +++ b/server/tests-py/context.py @@ -9,6 +9,7 @@ import queue import socket import subprocess +import time import yaml import requests @@ -40,6 +41,20 @@ def do_POST(self): self.server.error_queue.put({"path": req_path, "body": req_json, "headers": req_headers}) + elif req_path == "/timeout_small": + time.sleep(5) + self.send_response(HTTPStatus.NO_CONTENT) + self.end_headers() + self.server.error_queue.put({"path": req_path, + "body": req_json, + "headers": req_headers}) + elif req_path == "/timeout_long": + time.sleep(5) + self.send_response(HTTPStatus.NO_CONTENT) + self.end_headers() + self.server.resp_queue.put({"path": req_path, + "body": req_json, + "headers": req_headers}) else: self.send_response(HTTPStatus.NO_CONTENT) self.end_headers() @@ -47,7 +62,6 @@ def do_POST(self): "body": req_json, "headers": req_headers}) - class WebhookServer(http.server.HTTPServer): def __init__(self, resp_queue, error_queue, server_address): self.resp_queue = resp_queue diff --git a/server/tests-py/queries/event_triggers/retry_conf/setup.yaml b/server/tests-py/queries/event_triggers/retry_conf/setup.yaml index 5c4404dd8726b..c9d8b0e8223dc 100644 --- a/server/tests-py/queries/event_triggers/retry_conf/setup.yaml +++ b/server/tests-py/queries/event_triggers/retry_conf/setup.yaml @@ -7,10 +7,26 @@ args: c1 int, c2 text ); + create table hge_tests.test_t2( + c1 int, + c2 text + ); + create table hge_tests.test_t3( + c1 int, + c2 text + ); - type: track_table args: schema: hge_tests name: test_t1 +- type: track_table + args: + schema: hge_tests + name: test_t2 +- type: track_table + args: + schema: hge_tests + name: test_t3 - type: create_event_trigger args: name: t1_retry @@ -27,3 +43,37 @@ args: retry_conf: num_retries: 4 interval_sec: 2 +- type: create_event_trigger + args: + name: t2_timeout_small + table: + schema: hge_tests + name: test_t2 + insert: + columns: "*" + update: + columns: "*" + delete: + columns: "*" + webhook: http://127.0.0.1:5592/timeout_small + retry_conf: + num_retries: 2 + interval_sec: 2 + timeout_sec: 2 +- type: create_event_trigger + args: + name: t3_timeout_long + table: + schema: hge_tests + name: test_t3 + insert: + columns: "*" + update: + columns: "*" + delete: + columns: "*" + webhook: http://127.0.0.1:5592/timeout_long + retry_conf: + num_retries: 0 + interval_sec: 2 + timeout_sec: 10 diff --git a/server/tests-py/queries/event_triggers/retry_conf/teardown.yaml b/server/tests-py/queries/event_triggers/retry_conf/teardown.yaml index bb2bf6823e85f..f280d6a9a7a9d 100644 --- a/server/tests-py/queries/event_triggers/retry_conf/teardown.yaml +++ b/server/tests-py/queries/event_triggers/retry_conf/teardown.yaml @@ -6,4 +6,6 @@ args: - type: run_sql args: sql: | - drop table hge_tests.test_t1 + drop table hge_tests.test_t1; + drop table hge_tests.test_t2; + drop table hge_tests.test_t3; diff --git a/server/tests-py/test_events.py b/server/tests-py/test_events.py index c08925b7ac5c5..54709b5c52213 100755 --- a/server/tests-py/test_events.py +++ b/server/tests-py/test_events.py @@ -137,6 +137,32 @@ def test_basic(self, hge_ctx): tries = hge_ctx.get_error_queue_size() assert tries == 5, tries + def test_timeout_small(self, hge_ctx): + table = {"schema": "hge_tests", "name": "test_t2"} + + init_row = {"c1": 1, "c2": "hello"} + exp_ev_data = { + "old": None, + "new": init_row + } + st_code, resp = insert(hge_ctx, table, init_row) + assert st_code == 200, resp + time.sleep(20) + tries = hge_ctx.get_error_queue_size() + assert tries == 3, tries + + def test_timeout_long(self, hge_ctx): + table = {"schema": "hge_tests", "name": "test_t3"} + + init_row = {"c1": 1, "c2": "hello"} + exp_ev_data = { + "old": None, + "new": init_row + } + st_code, resp = insert(hge_ctx, table, init_row) + assert st_code == 200, resp + time.sleep(15) + check_event(hge_ctx, "t3_timeout_long", table, "INSERT", exp_ev_data, webhook_path = "/timeout_long") class TestEvtHeaders(object): From 7c21ddd58d5dbaba7992e8a46f8ce7ebc12a4f18 Mon Sep 17 00:00:00 2001 From: wawhal Date: Wed, 30 Jan 2019 18:20:28 +0530 Subject: [PATCH 7/9] create trigger with defaults if values not provided --- .../src/components/Services/EventTrigger/Add/AddState.js | 6 +++++- .../Services/EventTrigger/Modify/RetryConfEditor.js | 6 +++--- console/src/helpers/semver.js | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/console/src/components/Services/EventTrigger/Add/AddState.js b/console/src/components/Services/EventTrigger/Add/AddState.js index dbc13699ff2a5..46be29e3072b7 100644 --- a/console/src/components/Services/EventTrigger/Add/AddState.js +++ b/console/src/components/Services/EventTrigger/Add/AddState.js @@ -7,7 +7,11 @@ const defaultState = { selectedOperations: { insert: false, update: false, delete: false }, webhookURL: '', webhookUrlType: 'url', - retryConf: {}, + retryConf: { + num_retries: 0, + interval_sec: 10, + timeout_sec: 60, + }, ongoingRequest: false, lastError: null, internalError: null, diff --git a/console/src/components/Services/EventTrigger/Modify/RetryConfEditor.js b/console/src/components/Services/EventTrigger/Modify/RetryConfEditor.js index 5171bda67ae80..809c770cabe50 100644 --- a/console/src/components/Services/EventTrigger/Modify/RetryConfEditor.js +++ b/console/src/components/Services/EventTrigger/Modify/RetryConfEditor.js @@ -81,7 +81,7 @@ class RetryConfEditor extends React.Component { Number of retries:
- {retryConf.num_retries} + {retryConf.num_retries || 0}
@@ -89,14 +89,14 @@ class RetryConfEditor extends React.Component { Retry Interval (sec):
- {retryConf.interval_sec} + {retryConf.interval_sec || 10}
{supportRetryTimeout && (
Timeout (sec):
- {retryConf.timeout_sec} + {retryConf.timeout_sec || 60}
)} diff --git a/console/src/helpers/semver.js b/console/src/helpers/semver.js index 33e99c99ebfad..1bccf499c4ac3 100644 --- a/console/src/helpers/semver.js +++ b/console/src/helpers/semver.js @@ -14,7 +14,7 @@ const componentsSemver = { insertPermRestrictColumns: '1.0.0-alpha28', permHideUpsertSection: '1.0.0-alpha32', customFunctionSection: '1.0.0-alpha36', - triggerRetryTimeout: '1.0.0-alpha37', + triggerRetryTimeout: '1.0.0-alpha38', }; const getPreRelease = version => { From 3d13c5f64441a664c524d76b50a91ce54d518e89 Mon Sep 17 00:00:00 2001 From: wawhal Date: Thu, 31 Jan 2019 19:47:42 +0530 Subject: [PATCH 8/9] update logic for validating retry conf --- .../integration/validators/validators.js | 2 +- .../Services/EventTrigger/Add/AddActions.js | 21 +++++++-- .../Services/EventTrigger/Add/AddTrigger.js | 45 ++++++++++++------- .../Services/EventTrigger/Modify/Actions.js | 11 +++++ .../EventTrigger/Modify/RetryConfEditor.js | 34 +++++++++----- .../EventTrigger/Modify/WebhookEditor.js | 10 +++-- 6 files changed, 89 insertions(+), 34 deletions(-) diff --git a/console/cypress/integration/validators/validators.js b/console/cypress/integration/validators/validators.js index 4d827219d8a36..89935fac766fd 100644 --- a/console/cypress/integration/validators/validators.js +++ b/console/cypress/integration/validators/validators.js @@ -345,7 +345,7 @@ export const validateCTrigger = (triggerName, result) => { type: 'select', args: { table: { name: 'event_triggers', schema: 'hdb_catalog' }, - columns: ['*', 'table_name'], + columns: ['*'], where: { name: triggerName }, }, }; diff --git a/console/src/components/Services/EventTrigger/Add/AddActions.js b/console/src/components/Services/EventTrigger/Add/AddActions.js index d8887c30f59f8..19656a5a73405 100644 --- a/console/src/components/Services/EventTrigger/Add/AddActions.js +++ b/console/src/components/Services/EventTrigger/Add/AddActions.js @@ -115,6 +115,21 @@ const createTrigger = () => { payload.args.retry_conf = currentState.retryConf; } + payload.args.retry_conf = { + num_retries: + currentState.retryConf.num_retries === '' + ? 0 + : parseInt(currentState.retryConf.num_retries, 10), + interval_sec: + currentState.retryConf.interval_sec === '' + ? 10 + : parseInt(currentState.retryConf.interval_sec, 10), + timeout_sec: + currentState.retryConf.timeout_sec === '' + ? 60 + : parseInt(currentState.retryConf.timeout_sec, 10), + }; + // create header payload const headers = []; currentState.headers.map(header => { @@ -333,7 +348,7 @@ const addTriggerReducer = (state = defaultState, action) => { ...state, retryConf: { ...state.retryConf, - num_retries: parseInt(action.value, 10), + num_retries: action.value, }, }; case SET_RETRY_INTERVAL: @@ -341,7 +356,7 @@ const addTriggerReducer = (state = defaultState, action) => { ...state, retryConf: { ...state.retryConf, - interval_sec: parseInt(action.value, 10), + interval_sec: action.value, }, }; case SET_RETRY_TIMEOUT: @@ -349,7 +364,7 @@ const addTriggerReducer = (state = defaultState, action) => { ...state, retryConf: { ...state.retryConf, - timeout_sec: parseInt(action.value, 10), + timeout_sec: action.value, }, }; case SET_TABLENAME: diff --git a/console/src/components/Services/EventTrigger/Add/AddTrigger.js b/console/src/components/Services/EventTrigger/Add/AddTrigger.js index 23afa84e0b156..db41d4911a354 100644 --- a/console/src/components/Services/EventTrigger/Add/AddTrigger.js +++ b/console/src/components/Services/EventTrigger/Add/AddTrigger.js @@ -22,7 +22,7 @@ import { operationToggleAllColumns, setOperationSelection, setDefaults, - UPDATE_WEBHOOK_URL_TYPE + UPDATE_WEBHOOK_URL_TYPE, } from './AddActions'; import { listDuplicate } from '../../../../utils/data'; import { showErrorNotification } from '../Notification'; @@ -58,7 +58,7 @@ class AddTrigger extends Component { if (nextProps.serverVersion !== this.props.serverVersion) { this.checkSemVer(nextProps.serverVersion).then(() => { this.checkWebhookEnvSupport(nextProps.serverVersion); - this.checkRetryTimeoutSupport(this.props.serverVersion); + this.checkRetryTimeoutSupport(nextProps.serverVersion); }); } } @@ -99,7 +99,7 @@ class AddTrigger extends Component { updateSupportColumnChangeFeature(val) { this.setState({ - supportColumnChangeFeature: val + supportColumnChangeFeature: val, }); } @@ -130,23 +130,36 @@ class AddTrigger extends Component { errorMsg = 'Webhook URL cannot be empty'; customMsg = 'Webhook URL cannot be empty. Please add a valid URL'; } else if (this.props.retryConf) { - if (isNaN(parseInt(this.props.retryConf.num_retries, 10))) { + const iNumRetries = + this.props.retryConf.num_retries === '' + ? 0 + : parseInt(this.props.retryConf.num_retries, 10); + const iRetryInterval = + this.props.retryConf.interval_sec === '' + ? 10 + : parseInt(this.props.retryConf.interval_sec, 10); + const iTimeout = + this.props.retryConf.timeout_sec === '' + ? 60 + : parseInt(this.props.retryConf.timeout_sec, 10); + + if (iNumRetries < 0 || isNaN(iNumRetries)) { isValid = false; errorMsg = 'Number of retries is not valid'; - customMsg = 'Numer of retries cannot be empty and can only be numbers'; + customMsg = 'Numer of retries must be a non-negative number'; } - if (isNaN(parseInt(this.props.retryConf.interval_sec, 10))) { + if (iRetryInterval <= 0 || isNaN(iRetryInterval)) { isValid = false; errorMsg = 'Retry interval is not valid'; - customMsg = 'Retry interval cannot be empty and can only be numbers'; + customMsg = 'Retry interval must be a postiive number'; } if ( this.state.supportRetryTimeout && - isNaN(parseInt(this.props.retryConf.timeout_sec, 10)) + (isNaN(iTimeout) || iTimeout <= 0) ) { isValid = false; errorMsg = 'Timeout is not valid'; - customMsg = 'Timeout cannot be empty and can only be numbers'; + customMsg = 'Timeout must be a positive number'; } } else if (this.props.selectedOperations.insert) { // check if columns are selected. @@ -191,7 +204,7 @@ class AddTrigger extends Component { } else { this.props.dispatch( showErrorNotification('Error creating trigger!', errorMsg, '', { - custom: customMsg + custom: customMsg, }) ); } @@ -214,7 +227,7 @@ class AddTrigger extends Component { internalError, headers, webhookURL, - webhookUrlType + webhookUrlType, } = this.props; const { supportColumnChangeFeature, supportRetryTimeout } = this.state; @@ -320,7 +333,7 @@ class AddTrigger extends Component { className={styles.display_inline + ' ' + styles.add_mar_right} style={{ marginTop: '10px', - marginBottom: '10px' + marginBottom: '10px', }} > Applicable to update operation only. @@ -419,7 +432,7 @@ class AddTrigger extends Component { { return { ...state.addTrigger, schemaList: state.tables.schemaList, - serverVersion: state.main.serverVersion ? state.main.serverVersion : '' + serverVersion: state.main.serverVersion ? state.main.serverVersion : '', }; }; diff --git a/console/src/components/Services/EventTrigger/Modify/Actions.js b/console/src/components/Services/EventTrigger/Modify/Actions.js index 67e47d0295b6f..070fab30f6d38 100644 --- a/console/src/components/Services/EventTrigger/Modify/Actions.js +++ b/console/src/components/Services/EventTrigger/Modify/Actions.js @@ -6,6 +6,7 @@ import { loadProcessedEvents, } from '../EventActions'; import { UPDATE_MIGRATION_STATUS_ERROR } from '../../../Main/Actions'; +import { showErrorNotification } from '../Notification'; const SET_DEFAULTS = 'ModifyTrigger/SET_DEFAULTS'; export const setDefaults = () => ({ type: SET_DEFAULTS }); @@ -62,6 +63,16 @@ export const setHeaderValue = (data, index) => ({ export const REQUEST_ONGOING = 'ModifyTrigger/REQUEST_ONGOING'; export const REQUEST_COMPLETE = 'ModifyTrigger/REQUEST_COMPLETE'; +export const showValidationError = message => { + return dispatch => { + dispatch( + showErrorNotification('Error modifying trigger!', 'Invalid input', '', { + custom: message, + }) + ); + }; +}; + export const save = (property, triggerName) => { return (dispatch, getState) => { const { modifyTrigger } = getState(); diff --git a/console/src/components/Services/EventTrigger/Modify/RetryConfEditor.js b/console/src/components/Services/EventTrigger/Modify/RetryConfEditor.js index 809c770cabe50..2edb0973af869 100644 --- a/console/src/components/Services/EventTrigger/Modify/RetryConfEditor.js +++ b/console/src/components/Services/EventTrigger/Modify/RetryConfEditor.js @@ -1,7 +1,12 @@ import React from 'react'; import Editor from './Editor'; -import { setRetryNum, setRetryInterval, setRetryTimeout } from './Actions'; +import { + setRetryNum, + setRetryInterval, + setRetryTimeout, + showValidationError, +} from './Actions'; import Tooltip from './Tooltip'; import semverCheck from '../../../../helpers/semver'; @@ -45,26 +50,33 @@ class RetryConfEditor extends React.Component { retryConf: { numRetrys, retryInterval, timeout }, }, } = this.props; - if (isNaN(numRetrys)) { - alert('Number of retries should be an integer!'); + + const iNumRetries = numRetrys === '' ? 0 : parseInt(numRetrys, 10); + const iRetryInterval = + retryInterval === '' ? 10 : parseInt(retryInterval, 10); + const iTimeout = timeout === '' ? 60 : parseInt(timeout, 10); + + if (iNumRetries < 0 || isNaN(iNumRetries)) { + dispatch( + showValidationError('Number of retries must be a non negative number!') + ); return; } - dispatch(setRetryNum(parseInt(numRetrys, 10))); + dispatch(setRetryNum(iNumRetries)); - if (isNaN(retryInterval)) { - alert('Retry interval should be an integer!'); + if (iRetryInterval <= 0 || isNaN(iRetryInterval)) { + dispatch(showValidationError('Retry interval must be a postive number!')); return; } - dispatch(setRetryInterval(parseInt(retryInterval, 10))); + dispatch(setRetryInterval(iRetryInterval)); if (this.state.supportRetryTimeout) { - if (isNaN(retryInterval)) { - alert('Retry interval should be an integer!'); + if (isNaN(iTimeout) || iTimeout <= 0) { + dispatch(showValidationError('Timeout must be a positive number!')); return; } - dispatch(setRetryTimeout(parseInt(timeout, 10))); + dispatch(setRetryTimeout(iTimeout)); } - this.props.save(); }; diff --git a/console/src/components/Services/EventTrigger/Modify/WebhookEditor.js b/console/src/components/Services/EventTrigger/Modify/WebhookEditor.js index b13e09a79c831..da8641544aaa5 100644 --- a/console/src/components/Services/EventTrigger/Modify/WebhookEditor.js +++ b/console/src/components/Services/EventTrigger/Modify/WebhookEditor.js @@ -1,7 +1,11 @@ import React from 'react'; import Editor from './Editor'; import DropdownButton from '../../../Common/DropdownButton/DropdownButton'; -import { setWebhookUrl, setWebhookUrlType } from './Actions'; +import { + setWebhookUrl, + setWebhookUrlType, + showValidationError, +} from './Actions'; import Tooltip from './Tooltip'; class WebhookEditor extends React.Component { @@ -18,7 +22,7 @@ class WebhookEditor extends React.Component { }; validateAndSave = () => { - const { modifyTrigger } = this.props; + const { modifyTrigger, dispatch } = this.props; if (modifyTrigger.webhookUrlType === 'url') { let tempUrl = false; try { @@ -27,7 +31,7 @@ class WebhookEditor extends React.Component { console.error(e); } if (!tempUrl) { - alert('Invalid URL'); + dispatch(showValidationError('Invalid URL')); return; } } From 1787d08717003aaea821b43ecef5ee657744e108 Mon Sep 17 00:00:00 2001 From: Tirumarai Selvan A Date: Thu, 7 Feb 2019 20:22:31 +0530 Subject: [PATCH 9/9] rename timeout_small -> timeout_short --- server/tests-py/context.py | 2 +- server/tests-py/queries/event_triggers/retry_conf/setup.yaml | 4 ++-- server/tests-py/test_events.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/tests-py/context.py b/server/tests-py/context.py index d2173f3b74c8b..a34ed2379abb4 100644 --- a/server/tests-py/context.py +++ b/server/tests-py/context.py @@ -41,7 +41,7 @@ def do_POST(self): self.server.error_queue.put({"path": req_path, "body": req_json, "headers": req_headers}) - elif req_path == "/timeout_small": + elif req_path == "/timeout_short": time.sleep(5) self.send_response(HTTPStatus.NO_CONTENT) self.end_headers() diff --git a/server/tests-py/queries/event_triggers/retry_conf/setup.yaml b/server/tests-py/queries/event_triggers/retry_conf/setup.yaml index c9d8b0e8223dc..61e7d10c731ed 100644 --- a/server/tests-py/queries/event_triggers/retry_conf/setup.yaml +++ b/server/tests-py/queries/event_triggers/retry_conf/setup.yaml @@ -45,7 +45,7 @@ args: interval_sec: 2 - type: create_event_trigger args: - name: t2_timeout_small + name: t2_timeout_short table: schema: hge_tests name: test_t2 @@ -55,7 +55,7 @@ args: columns: "*" delete: columns: "*" - webhook: http://127.0.0.1:5592/timeout_small + webhook: http://127.0.0.1:5592/timeout_short retry_conf: num_retries: 2 interval_sec: 2 diff --git a/server/tests-py/test_events.py b/server/tests-py/test_events.py index 8da31c5a7a2cb..064ed602b4be8 100755 --- a/server/tests-py/test_events.py +++ b/server/tests-py/test_events.py @@ -140,7 +140,7 @@ def test_basic(self, hge_ctx): tries = hge_ctx.get_error_queue_size() assert tries == 5, tries - def test_timeout_small(self, hge_ctx): + def test_timeout_short(self, hge_ctx): table = {"schema": "hge_tests", "name": "test_t2"} init_row = {"c1": 1, "c2": "hello"}