Whitelist kea-typegen folder in .dockerignore #1443

mariusandra · 2020-08-17T10:55:37Z

Changes

I only saw after the PR was merged that in master all commits also get a docker test. This was failing. I hope the following change restores order to the republic.

Checklist

All querysets/queries filter by Team (if this PR affects any querysets/queries)
Backend tests (if this PR affects the backend)
Cypress E2E tests (if this PR affects the front and/or backend)

yakkomajuri · 2020-08-17T11:18:47Z

Added DEBUG to dev.Dockerfile since the lack of it was causing a crash after the secret key PR was merged. Thought it'd be better here than a separate PR

yakkomajuri · 2020-08-17T11:19:50Z

I'm still getting a Kea error though:

$ kea-typegen write && echo "Building Webpack" && NODE_ENV=production webpack --config webpack.config.js && cp -a frontend/public/* frontend/dist/ && npm run copy-array
/code/node_modules/kea-typegen/node_modules/yargs/build/lib/yargs.js:1132
                throw err;
                ^

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at validateString (internal/validators.js:120:11)
    at Object.dirname (path.js:1128:5)
    at includeKeaConfig (/code/node_modules/kea-typegen/dist/cli/typegen.js:54:32)
    at Object.handler (/code/node_modules/kea-typegen/dist/cli/typegen.js:15:44)
    at Object.runCommand (/code/node_modules/kea-typegen/node_modules/yargs/build/lib/command.js:196:48)
    at Object.parseArgs [as _parseArgs] (/code/node_modules/kea-typegen/node_modules/yargs/build/lib/yargs.js:1043:55)
    at Object.get [as argv] (/code/node_modules/kea-typegen/node_modules/yargs/build/lib/yargs.js:986:25)
    at Object.<anonymous> (/code/node_modules/kea-typegen/dist/cli/typegen.js:36:14)
    at Module._compile (internal/modules/cjs/loader.js:1137:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1157:10) {
  code: 'ERR_INVALID_ARG_TYPE'
}
error Command failed with exit code 1.

yakkomajuri · 2020-08-17T11:22:02Z

Different error following the latest commit:

Using TypeScript Config: /code/tsconfig.json

/code/node_modules/kea-typegen/node_modules/yargs/build/lib/yargs.js:1132
                throw err;
                ^

TypeError: Cannot read property 'length' of undefined
    at convertToObjectWorker (/code/node_modules/typescript/lib/typescript.js:28120:36)
    at parseOwnConfigOfJsonSourceFile (/code/node_modules/typescript/lib/typescript.js:28834:20)
    at parseConfig (/code/node_modules/typescript/lib/typescript.js:28741:13)
    at parseJsonConfigFileContentWorker (/code/node_modules/typescript/lib/typescript.js:28588:28)
    at Object.parseJsonSourceFileConfigFileContent (/code/node_modules/typescript/lib/typescript.js:28555:16)
    at runCLI (/code/node_modules/kea-typegen/dist/cli/typegen.js:115:36)
    at Object.handler (/code/node_modules/kea-typegen/dist/cli/typegen.js:15:5)
    at Object.runCommand (/code/node_modules/kea-typegen/node_modules/yargs/build/lib/command.js:196:48)
    at Object.parseArgs [as _parseArgs] (/code/node_modules/kea-typegen/node_modules/yargs/build/lib/yargs.js:1043:55)
    at Object.get [as argv] (/code/node_modules/kea-typegen/node_modules/yargs/build/lib/yargs.js:986:25)
error Command failed with exit code 1.

mariusandra · 2020-08-17T11:24:47Z

Let's try again :). I'm building docker-compose.e2e.yml locally, so it's taking some time before I see the error.

yakkomajuri · 2020-08-17T11:32:36Z

Yup, Mac starts hissing quite a bit...

yakkomajuri · 2020-08-17T11:32:57Z

Last commit worked for me!

yakkomajuri · 2020-08-17T11:34:10Z

But I built with dev.yml - You might get a secret key issue with e2e.yml - think I need to add DEBUG to production.Dockerfile too

mariusandra · 2020-08-17T11:36:10Z

Hm... For me it still kept giving errors even after the last changes. I think the last commit will fix it (still running locally to verify)

mariusandra · 2020-08-17T11:41:52Z

Now it built the frontend well, but gives this at the "collectstatic" step:

yakkomajuri · 2020-08-17T11:42:46Z

Try now

mariusandra · 2020-08-17T11:48:19Z

I'd do the same trick in production.Dockerfile as for preview.Dockerfile. The secret key is not used in the collectstatic phase. It's just there to keep django quiet. With DEBUG=1, when running the production dockerfile, I see such a warning in the logs, which will cause more confusion than it's worth:

yakkomajuri · 2020-08-17T11:53:00Z

I'm just wondering if it messes up some sort of cache...

Yesterday, after I had the containers built and changed the SECRET_KEY, it didn't update correctly in all services:

yakkomajuri · 2020-08-17T11:54:51Z

Those two pictures above are from the same run. It's something that might need to be looked at more in-depth. But one thing that might be useful for this and other scenarios is a SUPPRESS_WARNINGS var that would be checked in print_warning

mariusandra · 2020-08-17T11:57:29Z

Suppressing warnings can lead to all sorts of hard to track bugs...

In this case I'd just make a random secret key for collectstatic in both preview and production Dockerfiles and this PR should be good to go...

There are a bunch of broken Cypress tests again that I'm trying to resolve for PR #1424. I propose they shouldn't be a blocker here, as they happen in every branch :/.

mariusandra · 2020-08-17T11:59:31Z

Regarding those two screenshots, are you sure you changed the secret key for both the worker and the web containers?

yakkomajuri · 2020-08-17T12:00:13Z

No I'm not sure - It was pretty late and I probably missed. Think that was it.

yakkomajuri · 2020-08-17T12:01:29Z

I still think SUPPRESS_WARNINGS could be useful, especially given that all it would suppress is the DEBUG and the secret key warnings - It would also only be used in the setup process, like collectstatic

mariusandra · 2020-08-17T12:05:08Z

We don't know what warnings it will suppress in the future though :)... and I fear it'll be surely abused by some people to get rid of the "annoying" debug/key warnings in production without understanding the full story nor implementing a proper fix.

If needed, adding > /dev/null to the end of python manage.py collectstatic will have the same suppression effect... while letting errors (stderr) through.

yakkomajuri · 2020-08-17T12:06:01Z

Yup makes sense! And now that you've updated the keys, this should be okay? Deployment failed though...

yakkomajuri · 2020-08-17T12:07:37Z

I don't have access to Heroku so can't see what's going on

mariusandra · 2020-08-17T12:07:50Z

It failed in a weird place:

so I think nothing to do with this PR

yakkomajuri · 2020-08-17T12:08:30Z

Ok yeah - All Cypress tests are also failing so I guess it's completely unrelated

mariusandra · 2020-08-17T12:10:06Z

Those are failing in other PRs as well. Looking at what's happening, I think it might somehow be related to an antd version upgrade. I'll try to debug this next. That's outside the scope of this PR though.

yakkomajuri · 2020-08-17T12:10:52Z

Nice!

mariusandra · 2020-08-17T12:11:27Z

Can you approve the PR? :)

yakkomajuri

We're good now.

mariusandra · 2020-08-17T12:12:22Z

TIL: Github works as a chat as well :D

(apologies to the 47 people watching)

Whitelist kea-typegen folder in .dockerignore

1b03eaf

mariusandra requested a review from yakkomajuri August 17, 2020 10:56

timgl temporarily deployed to posthog-docker-kea-type-usktcz August 17, 2020 10:56 Inactive

Added debug to devdockerfile

57088f8

timgl temporarily deployed to posthog-docker-kea-type-usktcz August 17, 2020 11:17 Inactive

quote style fix

f33dc51

timgl temporarily deployed to posthog-docker-kea-type-usktcz August 17, 2020 11:18 Inactive

add .kearc to dockerignore (needed to generate types in yarn build)

1a791b4

mariusandra temporarily deployed to posthog-docker-kea-type-usktcz August 17, 2020 11:21 Inactive

Add tsconfig.json to dockerfile

5d3f6dd

mariusandra temporarily deployed to posthog-docker-kea-type-usktcz August 17, 2020 11:24 Inactive

add tsconfig and kearc to dockerfiles

6b4ab56

mariusandra temporarily deployed to posthog-docker-kea-type-usktcz August 17, 2020 11:36 Inactive

updated docker files to suppress secret key error

a32d15b

timgl deployed to posthog-docker-kea-type-usktcz August 17, 2020 11:43 View deployment

use fake secret key in collectstatic

9ac9c00

mariusandra temporarily deployed to posthog-docker-kea-type-usktcz August 17, 2020 12:00 Inactive

yakkomajuri approved these changes Aug 17, 2020

View reviewed changes

mariusandra merged commit f1c90f4 into master Aug 17, 2020

mariusandra deleted the docker-kea-typegen-ignore-fix branch August 17, 2020 12:11

mariusandra mentioned this pull request Aug 17, 2020

Revert "Update dependencies (#1436)" #1444

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Whitelist kea-typegen folder in .dockerignore #1443

Whitelist kea-typegen folder in .dockerignore #1443

mariusandra commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020 •

edited

Loading

yakkomajuri commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

mariusandra commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

mariusandra commented Aug 17, 2020

mariusandra commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

mariusandra commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

mariusandra commented Aug 17, 2020

mariusandra commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

mariusandra commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

mariusandra commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

mariusandra commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

mariusandra commented Aug 17, 2020

yakkomajuri left a comment

mariusandra commented Aug 17, 2020

Whitelist kea-typegen folder in .dockerignore #1443

Whitelist kea-typegen folder in .dockerignore #1443

Conversation

mariusandra commented Aug 17, 2020

Changes

Checklist

yakkomajuri commented Aug 17, 2020 • edited Loading

yakkomajuri commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

mariusandra commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

mariusandra commented Aug 17, 2020

mariusandra commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

mariusandra commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

mariusandra commented Aug 17, 2020

mariusandra commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

mariusandra commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

mariusandra commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

mariusandra commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020

mariusandra commented Aug 17, 2020

yakkomajuri left a comment

Choose a reason for hiding this comment

mariusandra commented Aug 17, 2020

yakkomajuri commented Aug 17, 2020 •

edited

Loading