bacpop-25 use gps v6 db #42

EmmaLRussell · 2023-10-10T16:33:57Z

Target beebop_py branch which uses gps v6 db, and use v6 location.

Also a few changes related to intermittent test failures (described inline) - one still seems to be an issue, but not when I'm watching... 🙈

To be reviewed in conjunction with the corresponding beebop_py branch: bacpop/beebop_py#32

codecov-commenter · 2024-01-27T20:30:39Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b92fb8d) 98.91% compared to head (f5159bb) 98.91%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #42   +/-   ##
=======================================
  Coverage   98.91%   98.91%           
=======================================
  Files          31       31           
  Lines         648      648           
  Branches      105      105           
=======================================
  Hits          641      641           
  Misses          7        7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

EmmaLRussell · 2024-01-29T17:10:42Z

.github/workflows/jestCI.yml

@@ -50,7 +50,7 @@ jobs:
      with:
        node-version: ${{ matrix.node-version }}
        cache: 'npm'
-        cache-dependency-path: ./app/client/package-lock.json
+        cache-dependency-path: ./app/server/package-lock.json


This path just seemed to be wrong!

EmmaLRussell · 2024-01-29T17:15:12Z

app/client/tests/unit/components/SavedProjects.spec.ts


 const { toLocaleString } = Date.prototype;
-// eslint-disable-next-line no-extend-native
+// eslint-disable-next-line no-extend-native, func-names


Disabling this rule here to remove lint warning .

EmmaLRussell · 2024-01-29T17:15:35Z

app/server/tests/integration/errors.test.ts

@@ -43,19 +44,18 @@ describe("Error handling", () => {
        testSample.projectId = projectId;
        const poppunkRes = await post(`poppunk`, testSample, connectionCookie);
        expect(poppunkRes.status).toBe(200);
-        const counter = 0;
+        let counter = 0;


This counter hadn't been counting!

EmmaLRussell · 2024-01-29T17:17:05Z

app/server/tests/integration/errors.test.ts

-            await new Promise(resolve => {
-                setTimeout(() => {resolve(""), 1000})
-            });
+            await setTimeout(2000);


The previous attempt at delays between polling hadn't been working, so we were getting some intermittent test failures, which I think were related to spamming the endpoint. Bumping up the delay as well to avoid intermittent errors on first poll .

can we use fake timers here instead if possible?

Unfortunately not, as we're waiting for a popunk job to finish and polling the real beebop_py api.

EmmaLRussell · 2024-01-30T09:46:17Z

app/server/tests/integration/errors.test.ts

            const statusRes = await post("status", {hash: testSample.projectHash}, connectionCookie);
-            expect(statusRes.status).toBe(200);
+            // This is occasionally mysteriously flaky on CI and I haven't been able to diagnose why - surface the


absternator · 2024-01-31T09:39:30Z

scripts/run_dependencies

@@ -2,7 +2,7 @@ NETWORK=beebop_nw
 VOLUME=beebop-storage
 NAME_REDIS=beebop-redis
 NAME_API=beebop-py-api
-API_BRANCH=main
+API_BRANCH=bacpop-25


leaving comment so can be changed before merge

absternator · 2024-01-31T14:56:28Z

LGTM and worked well

richfitz · 2024-01-30T10:40:31Z

scripts/run_dependencies

       -v $VOLUME:/beebop/storage \
       mrcide/beebop-py:$API_BRANCH rqworker
-docker run -d --rm --name $NAME_API --network=$NETWORK \
-       --pull always \


should this pull not be retained? also the --rm?

The rm should! I'll re-add that. I've moved the pull to previous docker run, which is for the same image - mrcide/beebop-py:$API_BRANCH

richfitz · 2024-02-05T15:27:08Z

scripts/run_dependencies

       --env=REDIS_HOST="$NAME_REDIS" \
       --env=STORAGE_LOCATION="./storage" \
-       --env=DB_LOCATION="./storage/GPS_v4_references" \
+       --env=DB_LOCATION="./storage/GPS_v6_references" \


will we ever run with v4 again?

Probably not. Are you thinking we could just have a fixed storage location?

EmmaLRussell · 2024-02-06T14:46:00Z

This PR: #46 adds a docker quick start and includes commits from this branch too - may make it easier to test!

… error responses while polling

bacpop-145 Docker quick run

EmmaLRussell · 2024-02-19T16:02:03Z

app/server/tests/integration/errors.test.ts

+        const fakeProjectHash = `${projectId}ABC`;
+        const projectData = testSample(fakeProjectHash, projectId);
+        const poppunkRes = await post(`poppunk`, projectData, connectionCookie);


So I think part of the problem was that we weren't using a unique project hash every time we ran poppunk, so I've changed testSample from an object to a function, and making a new hash for each new project id...

EmmaLRussell · 2024-02-19T16:03:15Z

app/server/jest.config.js

@@ -2,5 +2,6 @@
 module.exports = {
  preset: 'ts-jest',
  testEnvironment: 'node',
-  testTimeout: 120000
+  testTimeout: 120000,
+  maxWorkers: 1


...but I think the main issue was that the persistence tests were blitzing redis, and they were potentially running in parallel with the error tests. So limiting to sequential run here...

EmmaLRussell added 2 commits October 10, 2023 17:32

use v6 db

b803baf

pull api image on first run

1e74854

EmmaLRussell mentioned this pull request Jan 29, 2024

bacpop-25 Use GPS_v6 db bacpop/beebop_py#32

Merged

EmmaLRussell added 6 commits January 29, 2024 10:50

squash lint warning in test file

72d966f

correct cache dependency path

a0d8b57

expect 404 on unexpected sample

44065ef

poll more sensibly in test

8234376

debug

2405525

bump up timeout

5f345a7

EmmaLRussell commented Jan 29, 2024

View reviewed changes

EmmaLRussell added 7 commits January 29, 2024 17:20

missing ;

095143d

add some extra logging to problem test

1d46810

logging fix

10e7225

throw if any error status responses

790eb67

more logging

189cea9

remove previous

de3e888

comment

97ffd74

EmmaLRussell commented Jan 30, 2024

View reviewed changes

EmmaLRussell marked this pull request as ready for review January 30, 2024 09:59

EmmaLRussell requested review from richfitz and absternator January 30, 2024 10:11

absternator reviewed Jan 31, 2024

View reviewed changes

EmmaLRussell added 4 commits February 5, 2024 10:50

updates for docker run

000c6da

rollback erroneous changes, and revert api branch pin

f34e7ce

add github docker env vars

c6e2223

fix broken server container

a7bd996

richfitz reviewed Feb 5, 2024

View reviewed changes

added docker oauth app for github

ab366fd

EmmaLRussell added 3 commits February 6, 2024 14:17

null check

d37f9b3

polling debug

943648f

remove debug

bda569e

EmmaLRussell added 3 commits February 6, 2024 15:22

timeout

2332bae

bump up universal timeout but do not allow more than five consecutive…

0ca15a3

… error responses while polling

show details of latest error response when exceed consecutive threshold

9a070c0

EmmaLRussell requested review from richfitz and absternator February 6, 2024 17:44

absternator approved these changes Feb 7, 2024

View reviewed changes

EmmaLRussell and others added 16 commits February 7, 2024 15:52

update for latest docker compose

f344503

Merge pull request #46 from bacpop/bacpop-145

12673ff

bacpop-145 Docker quick run

Merge branch 'main' into bacpop-25

9ddfa88

fix smoke test

67a3389

re-add vars to docker-compose

90c7319

use more informative py branch

ac27364

check assign job id on submit

758e8a5

revert api

ae44116

use bacpop-151

4bb8a94

use debug branch of py and include tmate

1036aa3

try putting tmate on the right workflow...

2060c5f

relax test for specific error string

f5159bb

use unique project hash for each test#

8a74952

use a single worker

12dc93e

remove allow status failures and target main py branch

248458b

remove tmate

c5a050c

EmmaLRussell commented Feb 19, 2024

View reviewed changes

richfitz approved these changes Feb 20, 2024

View reviewed changes

EmmaLRussell merged commit 681489b into main Feb 20, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bacpop-25 use gps v6 db #42

bacpop-25 use gps v6 db #42

EmmaLRussell commented Oct 10, 2023 •

edited

Loading

codecov-commenter commented Jan 27, 2024 •

edited

Loading

EmmaLRussell Jan 29, 2024

EmmaLRussell Jan 29, 2024

EmmaLRussell Jan 29, 2024

EmmaLRussell Jan 29, 2024 •

edited

Loading

absternator Jan 31, 2024

EmmaLRussell Feb 5, 2024

EmmaLRussell Jan 30, 2024

absternator Jan 31, 2024

absternator commented Jan 31, 2024

richfitz Jan 30, 2024

EmmaLRussell Feb 6, 2024

richfitz Feb 5, 2024

EmmaLRussell Feb 6, 2024

EmmaLRussell commented Feb 6, 2024

EmmaLRussell Feb 19, 2024

EmmaLRussell Feb 19, 2024

bacpop-25 use gps v6 db #42

bacpop-25 use gps v6 db #42

Conversation

EmmaLRussell commented Oct 10, 2023 • edited Loading

codecov-commenter commented Jan 27, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EmmaLRussell Jan 29, 2024 • edited Loading

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

absternator commented Jan 31, 2024

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

EmmaLRussell commented Feb 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EmmaLRussell commented Oct 10, 2023 •

edited

Loading

codecov-commenter commented Jan 27, 2024 •

edited

Loading

EmmaLRussell Jan 29, 2024 •

edited

Loading