Added workflow and gcp JS client library script with package json #2889

DiegoManzanarezx · 2022-07-19T18:14:45Z

The purpose of this draft PR is to add a Github actions workflow that will be executed every time there is a push to this repository or when manually triggering it through Github UI.
The workflow will run a Javascript Google Cloud Client library file that will create and send a task to a Google Cloud Tasks Queue and will then forward it to a Google Cloud Workflow that will execute the test runner.

This draft PR consists on adding 3 new files:

A Github actions yaml file to install all the dependencies needed to successfully run a Javascript file that implements Google Cloud Tasks Queue Client library.
A Javascript file that create and send a task to a Queue, using Google Cloud Tasks Queue Client library.
A package.json file that stores all the dependencies needed for the JS file.

*To verify the reliability of the workflow we have first built and tested a fully functional prototype that works as expected.

Considerations:
a) In order to execute the workflow we need credentials and other information (i.e. the name of our project in Google Cloud Platform) that we will provide, but must remain confidential, therefore, those should be stored as Github Actions secrets and are the following:
GOOGLE_APPLICATION_CREDENTIALS -> This is a JSON file that will authenticate our GCP account at every execution.
LOCATION -> Location where the Queue is running.
PROJECT_ID -> The name of the GCP project where the Queue is stored.
QUEUE_ID -> The name of the Queue where the task will be sent.
SERVICE_ACCOUNT_ROLE -> The GCP account that contains the permissions and roles to manage the Queue.
WORKFLOW_URL -> The endpoint to which the Queue will send an http request to forward the task to a GCP Workflow.

b) The purpose of triggering manually the workflow is to run personalized tests, so 3 inputs will we requested:
- Commit hash. (required)
- Classes to test. (optional)
- Pull request number. (optional)
*If the optional fields are left empty, all tests will be executed on that specific commit.
We will comment a link to the tests results on this pull request.

c) Since we are still working on the test runner this draft PR is intended just for code reviews purposes. We plan on landing the PR once the project is on a working state, probably about four weeks time.

chriswr95

Please also add a detailed description to the PR. What is this code? What purpose does it serve? How did we test it? We also want to communicate that we're raising the PR now so we can code review it, but don't plan on landing the PR until later in the project. Until then, we're hoping it can hang out as a draft. Probably about four weeks time.

chriswr95 · 2022-07-19T18:19:04Z

.github/workflows/gql-perf-arch-github_workflow.yml

+        description: 'Classes to test'
+        required: false
+      PULL_REQUEST_NUMBER_INPUT:
+        description: 'Pull request number'


can you the usage of each of these parameters to the description? for example if this is specified, we will comment a link to the tests results on this pull request.

I added it to the description.

chriswr95 · 2022-07-19T18:19:19Z

.github/workflows/gql-perf-arch-github_workflow.yml

+  #Actions secrets (keys)
+  PROJECT_ID: ${{ secrets.PROJECT_ID }}
+  QUEUE_ID: ${{ secrets.QUEUE_ID }}
+  LOCATION: ${{ secrets.LOCATION }}


what is this?

chriswr95 · 2022-07-19T18:19:31Z

.github/workflows/gql-perf-arch-github_workflow.yml

+  QUEUE_ID: ${{ secrets.QUEUE_ID }}
+  LOCATION: ${{ secrets.LOCATION }}
+  WORKFLOW_URL: ${{ secrets.WORKFLOW_URL }}
+  SERVICE_ACCOUNT_EMAIL: ${{ secrets.SERVICE_ACCOUNT_EMAIL }}


why is this needed?

I already updated the description of the draft PR to explain why we need each of those variables.

chriswr95 · 2022-07-19T18:20:12Z

.github/workflows/gql-perf-arch-github_workflow.yml

+      - run: npm i yenv
+      - run: npm i uuid
+
+      - id: 'auth'


this is very cool

chriswr95 · 2022-07-19T18:24:03Z

google_cloud_http_target_task.js

+  const serviceAccountEmail = parsedDocument.SERVICE_ACCOUNT_EMAIL;
+
+  // Construct payload
+  const payloadStructure =  {


please format this like

const payloadStructure = { val1: var1, val2: var2, ... };

chriswr95 · 2022-07-19T18:27:48Z

google_cloud_http_target_task.js

+  console.log(`Created task ${name}`);
+  console.log(`Your job id is: ${jobId}`);
+}
+createHttpTaskWithToken();


let's chat in person about refactoring this script just a bit, overall it looks pretty good.

chriswr95 · 2022-07-19T18:28:55Z

.github/workflows/gql-perf-arch-github_workflow.yml

+        with:
+          node-version: '14'
+      - run: npm install
+      - run: npm i yenv


if you add yenv to your package.json, can you skip this extra step, as it will be installed during npm install?

Done, I added it to the package.json instead.

chriswr95 · 2022-07-19T18:30:46Z

package.json

+  "devDependencies": {
+    "chai": "^4.2.0",
+    "mocha": "^8.0.0",
+    "uuid": "^8.0.0"


is uuid only a dev dependency? I think it should be a regular dependency. Similarly, do you need mocha and chai? I we don't have any tests for this code.

Yes, I have already updated this file. We don't need mocha an chai.

I am not really comfortable with having this in the root folder. Can we move this file into the .github/workflow directory?
The reason is security and maintenance: this is not s JS project and we don't depend on JS libraries for the actual lib we publish. This should be clearly only used for the github workflow

Thanks for that Andi! I moved it into workflows folder.

chriswr95 · 2022-07-19T18:31:43Z

google_cloud_http_target_task.js

@@ -0,0 +1,63 @@
+const {CloudTasksClient} = require('@google-cloud/tasks');


this probably shouldn't live at the root level, maybe instead at .github/workflow-scripts?

Done. I added it inside .github/workflow

chriswr95 · 2022-07-19T18:32:46Z

google_cloud_http_target_task.js

+const yenv = require('yenv')
+const { v4: uuidv4 } = require('uuid');
+const fs = require('fs')
+const os = require('os');


is this used anywhere?

No, I have already fixed it in the new commit.

adarsh-jaiswal · 2022-07-20T18:15:24Z

.github/workflows/google_cloud_http_target_task.js

+const yenv = require('yenv')
+const { v4: uuidv4 } = require('uuid');
+
+const parsedDocument = yenv('.github/workflows/gql-perf-arch-github_workflow.yml')


Is it possible to use process.env.COMMIT_HASH and likewise for all other env variables? we might not need yenv dependency.

Yes, you were right. The change is done.

chriswr95

This code looks much tighter.

NIT: For your PR description, you are now a little bit too verbose. Aim for clear and succinct language. You should spend 20 minutes writing the description so that a reviewer can read it in one minute. Using Markdown also helps to structure longer messages. No need to change anything, just leaving this for the future :)

Approved pending a final nit comment.

chriswr95 · 2022-07-21T19:15:04Z

.github/workflows/google_cloud_http_target_task.js

+}
+
+async function createHttpTaskWithToken() {
+  const payloadStructure = constructPayload();


you can wrap lines 58 - 61 into yet another helper function createRequest() which returns the request.

chriswr95 · 2022-07-21T19:16:02Z

.github/workflows/gql-perf-arch-github_workflow.yml

+        description: 'Pull request number'
+        required: false
+env:
+  #Actions secrets (keys)


can remove this comment.

… onf yenv

andimarek · 2022-09-01T22:54:11Z

package.json

+  "dependencies": {
+    "@google-cloud/tasks": "^3.0.0",
+    "body-parser": "^1.18.3",
+    "express": "^4.16.3",


do we need express here? I don't think we are running any server, right?

True, we don't need express. Already removed it.

andimarek · 2022-09-01T23:04:18Z

package.json

+  "name": "appengine-cloudtasks",
+  "description": "Google App Engine Cloud Tasks example.",
+  "license": "Apache-2.0",
+  "author": "Google Inc.",


I don't think this is right. We could just remove author, license, descp etc as we don;t plan to publish this

True, I removed those fields.

Added workflow and gcp JS client library script with package json

b42aab6

chriswr95 reviewed Jul 19, 2022

View reviewed changes

Diego Manzanarez added 2 commits July 20, 2022 10:32

Refactored code

a5eb181

Fixed branch field in yml file and dependencies in package json

6a302e9

adarsh-jaiswal reviewed Jul 20, 2022

View reviewed changes

chriswr95 approved these changes Jul 21, 2022

View reviewed changes

Diego Manzanarez added 3 commits July 22, 2022 10:45

Added createRequest function and switched over to process env instead…

a43e57b

… onf yenv

Added string to array function

8461156

Fixed typos on string to array function

e7ff815

DiegoManzanarezx marked this pull request as ready for review August 18, 2022 22:52

adarsh-jaiswal mentioned this pull request Aug 24, 2022

Summary firestore data chriswr95/graphql-java-test-runner#17

Merged

andimarek reviewed Sep 1, 2022

View reviewed changes

Moved package json to workflows folder and removed unused dependencies

03776d7

andimarek merged commit 74189e0 into graphql-java:master Oct 18, 2022

		@@ -0,0 +1,63 @@
		const {CloudTasksClient} = require('@google-cloud/tasks');

Added workflow and gcp JS client library script with package json #2889

Added workflow and gcp JS client library script with package json #2889

Uh oh!

Conversation

DiegoManzanarezx commented Jul 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chriswr95 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andimarek Sep 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adarsh-jaiswal Jul 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chriswr95 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

DiegoManzanarezx commented Jul 19, 2022 •

edited

Loading

andimarek Sep 1, 2022 •

edited

Loading

adarsh-jaiswal Jul 20, 2022 •

edited

Loading