Feature/es support server #291

just-at-uber · 2021-04-23T23:19:05Z

This PR is changes required by Cadence UI server to fetch data from admin service for DescribeCluster.

Because DescribeCluster is in admin.thrift, a new channel is needed to be setup (admin channel). This is the change made to makeChannel to return 2 channels which all other requests will use cadence channel. the new cluster handler will use the admin channel.

membershipInfo will be large in size in production so I have nulled that field for the sake of saving on size of request when making this API call. This response will be saved temporarily on the node server for 1 hour in order to save on hitting cadence server too hard as this data will not change very often (can be adjusted in router/constants.js).

Added

clusterHandler to return cluster data from cadence server. Caches data for 1 hour.

Changed

makeChannel renamed to makeChannels and now returns 2 channels. returns admin and cadence channels.
channelName can now be passed in as an option to request. Defaults to cadence.
serviceName can now be passed in as an option to request. Defaults to WorkflowService.

…plit out more helpers in router into helpers.

…plete response now available from UI. need to cache response on server side and trim what is stored.

just-at-uber · 2021-04-23T23:30:41Z

server/router/routes/cluster-handler.js

+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.


clusterHandler to return cluster data from cadence server. Caches data for 1 hour.

just-at-uber · 2021-04-23T23:30:56Z

server/router/index.js

@@ -45,7 +46,10 @@ const { listWorkflows } = require('./helpers');

 const router = new Router();

+router.get('/api/cluster', clusterHandler);


clusterHandler to return cluster data from cadence server. Caches data for 1 hour.

just-at-uber · 2021-04-23T23:31:08Z

server/router/routes/index.js

@@ -19,6 +19,7 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.

+const clusterHandler = require('./cluster-handler');


clusterHandler to return cluster data from cadence server. Caches data for 1 hour.

just-at-uber · 2021-04-23T23:31:49Z

server/router/constants.js

+// THE SOFTWARE.
+
+const ONE_HOUR_IN_MILLISECONDS = 60 * 60 * 1000;
+const CLUSTER_CACHE_TTL = ONE_HOUR_IN_MILLISECONDS;


Caches data for 1 hour.

just-at-uber · 2021-04-23T23:32:17Z

server/middleware/tchannel-client/index.js

+      method: 'DescribeCluster',
+      requestName: 'describe',
+      serviceName: 'AdminService',
+    }),


clusterHandler to return cluster data from cadence server. Caches data for 1 hour.

just-at-uber · 2021-04-23T23:32:42Z

server/middleware/tchannel-client/index.js

@@ -37,10 +37,10 @@ const tchannelClient = async function(ctx, next) {
  const { authTokenHeaders = {} } = ctx;

  const client = TChannel();
-  const channel = await makeChannel(client);
+  const channels = await makeChannels(client);


makeChannel renamed to makeChannels and now returns 2 channels.

just-at-uber · 2021-04-23T23:33:03Z

server/middleware/tchannel-client/helpers/make-request.js

 }) => body =>
  new Promise((resolve, reject) => {
    try {
-      channel.request(REQUEST_CONFIG).send(
-        formatMethod(method),
+      channels[channelName].request(REQUEST_CONFIG).send(


makeChannel renamed to makeChannels and now returns 2 channels.

just-at-uber · 2021-04-23T23:33:18Z

server/middleware/tchannel-client/helpers/make-request.js

  responseTransform,
+  serviceName = 'WorkflowService',


serviceName can now be passed in as an option to request. Defaults to WorkflowService.

just-at-uber · 2021-04-23T23:33:32Z

server/middleware/tchannel-client/helpers/make-request.js

-const makeRequest = ({ authTokenHeaders, channel, ctx }) => ({
+const makeRequest = ({ authTokenHeaders, channels, ctx }) => ({
+  bodyTransform,
+  channelName = 'cadence',


channelName can now be passed in as an option to request. Defaults to cadence.

just-at-uber · 2021-04-23T23:34:08Z

server/middleware/tchannel-client/helpers/make-channels.js

@@ -25,7 +25,7 @@ const TChannelAsThrift = require('tchannel/as/thrift');
 const { PEERS } = require('../constants');
 const lookupAsync = require('./lookup-async');

-const makeChannel = async client => {
+const makeChannels = async client => {


makeChannel renamed to makeChannels and now returns 2 channels.

just-at-uber · 2021-04-23T23:34:16Z

server/middleware/tchannel-client/helpers/index.js

@@ -38,7 +38,7 @@ module.exports = {
  formatMethod,
  formatRequestName,
  lookupAsync,
-  makeChannel,
+  makeChannels,


makeChannel renamed to makeChannels and now returns 2 channels.

just-at-uber · 2021-04-23T23:34:30Z

server/middleware/tchannel-client/helpers/format-method.js

@@ -19,6 +19,6 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.

-const formatMethod = method => `WorkflowService::${method}`;
+const formatMethod = ({ method, serviceName }) => `${serviceName}::${method}`;


serviceName can now be passed in as an option to request. Defaults to WorkflowService.

mkolodezny · 2021-04-26T20:49:36Z

server/middleware/tchannel-client/helpers/make-request.js

@@ -25,16 +25,18 @@ const formatMethod = require('./format-method');
 const formatRequestName = require('./format-request-name');
 const uiTransform = require('./ui-transform');

-const makeRequest = ({ authTokenHeaders, channel, ctx }) => ({
+const makeRequest = ({ authTokenHeaders, channels, ctx }) => ({


do we use the same auth token for the both channels?

It's a good question. I need to be able to test this in staging to know whether we need different credentials. Right now I'm not sure.

It shouldn't be a massive change in the event it does need separate credentials.

mkolodezny · 2021-04-26T20:50:46Z

server/router/routes/cluster-handler.js

+
+let cache = null;
+
+const clusterHandler = async ctx => {


is the handler exposed somewhere on UI?

It will be eventually, right now this is only containing the server side code changes. will raise separate PR which hooks this up in the frontend.

mkolodezny

LGTM, just few minor comments.

…tests

just-at-uber · 2021-04-27T18:29:10Z

#292 will fix failing integration tests. Merged into current PR to ensure this fixes failing tests and builds are green.

just-at-uber added 16 commits April 22, 2021 16:47

refactoring server

b2457b9

more refactoring

aec6f41

fix lint

896cb5a

moved router into its own folder. moved helpers into router folder. s…

4e8de81

…plit out more helpers in router into helpers.

Merge branch 'master' into feature/es-support

12989de

fix conflicts

6952a2e

moving code into separate files for route handlers

c17d10a

fix lint

366f3a5

Merge branch 'master' into feature/es-support

91ce070

adding cluster handler

dff0856

fix lint

59dd932

piped through tchannel changes to support admin service requests. com…

5117b29

…plete response now available from UI. need to cache response on server side and trim what is stored.

adding caching to cluster handler

9a80a84

adding cluster cache ttl constant

5447c1b

revert frontend changes

ccd8f49

fix lint

21eeb7e

just-at-uber commented Apr 23, 2021

View reviewed changes

just-at-uber marked this pull request as ready for review April 26, 2021 17:47

just-at-uber requested a review from a team April 26, 2021 17:47

artificial change to test integration tests.

1412c2f

mkolodezny reviewed Apr 26, 2021

View reviewed changes

mkolodezny approved these changes Apr 26, 2021

View reviewed changes

just-at-uber added 18 commits April 26, 2021 16:23

Merge branch 'master' into feature/es-support-server

bdd718a

Merge branch 'master' into feature/es-support-server

9dbcd34

revert readme

6536ad7

Merge branch 'feature/es-support-server' into fix/flakey-integration-…

a1520d0

…tests

adding timeout

a7f0b27

add done for async test

d2587f0

reducing timeout to 500ms

042ce11

fixed timeout as this is the timeout for the test before failing

c57c35e

increasing timeout time

73173aa

removing done function as it does not work

9e4321e

increase timeout to 20000ms

466d1ac

increasing timeout to 50K ms

7548afd

scrapping timeout and inserting delay to test

d9a1a9a

tests now passing. trying to reduce timeout for faster runtime

61f28b5

increasing delay to 200ms

c6d59b9

increasing delay to 300ms

ea4b459

increasing to 500ms

b91ddf3

increasing delay to 1000ms

153436d

just-at-uber merged commit c6c2b0b into master Apr 27, 2021

just-at-uber deleted the feature/es-support-server branch April 27, 2021 18:34

just-at-uber mentioned this pull request Apr 27, 2021

Fix/flakey integration tests #292

Closed

just-at-uber mentioned this pull request May 27, 2021

v3.26.0 #332

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/es support server #291

Feature/es support server #291

just-at-uber commented Apr 23, 2021 •

edited

Loading

just-at-uber Apr 23, 2021

just-at-uber Apr 23, 2021

just-at-uber Apr 23, 2021

just-at-uber Apr 23, 2021

just-at-uber Apr 23, 2021

just-at-uber Apr 23, 2021

just-at-uber Apr 23, 2021

just-at-uber Apr 23, 2021

just-at-uber Apr 23, 2021

just-at-uber Apr 23, 2021

just-at-uber Apr 23, 2021

just-at-uber Apr 23, 2021

mkolodezny Apr 26, 2021

just-at-uber Apr 26, 2021

just-at-uber Apr 26, 2021

mkolodezny Apr 26, 2021

just-at-uber Apr 26, 2021

mkolodezny left a comment

just-at-uber commented Apr 27, 2021 •

edited

Loading

		@@ -45,7 +46,10 @@ const { listWorkflows } = require('./helpers');

		const router = new Router();

		router.get('/api/cluster', clusterHandler);

Feature/es support server #291

Feature/es support server #291

Conversation

just-at-uber commented Apr 23, 2021 • edited Loading

Added

Changed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkolodezny left a comment

Choose a reason for hiding this comment

just-at-uber commented Apr 27, 2021 • edited Loading

just-at-uber commented Apr 23, 2021 •

edited

Loading

just-at-uber commented Apr 27, 2021 •

edited

Loading