Shields: Docker issue: production code imports test dependencies

[calebcartwright]

I just did a deploy of the internal Shields instance we host at my day job and ran into a couple dev vs. prod dependency issues. I had to move the below dependencies from dev to prod deps in order to get the container to run:

• nock
• icedfrisby
• icedfrisby-nock
• caller

Everything worked fine afterwards. I haven't had a chance to dig into the underlying cause yet, but I wouldn't have expected those to be runtime/prod dependencies

[paulmelnikow]

Ah… caller is a good clue, since we only import it in one place, which is create-service-tester. After #2809 we import create-service-tester from services/index.js, and that's imported by all the services. Methinks we're going to need to revisit that.

Calls for building some Docker validation sooner rather than later… would have been nice to have caught that earlier in the process (before doing so much work).

[calebcartwright]

That makes sense. Should we close this and track it under #2816 then?

[paulmelnikow]

It’s separate from fixing the immediate issue. We could track them separately.

[calebcartwright]

Fair enough. I'm personally working around this by running an npm uninstall --save-dev and than npm i --save on those 4 dependencies in my build process prior to building the Docker image.

I'm assuming we want reconsider whether create-service-tester gets imported which sounds like a bigger fix vs. doing a short term fix by adding those 4 deps as prod dependencies?

If anyone else is having problems running the image we could point them here to run those two npm commands prior to the docker build ... command.

[paulmelnikow]

Another workaround is to use the commit before that one, temporarily.

I don’t think we should make these prod dependencies. I don’t think it needs to take a long time to fix the problem.

[calebcartwright]

Sounds good (and agreed)

[paulmelnikow]

Thoughts on how to fix this?

I’m reluctant to add more magic here, like conditional imports or try/catch imports. How about moving the test shortcut imports from services/index.js to services/tester.js, and updating all the tester imports to reference that instead?

[calebcartwright]

I like that idea. I'll take a shot at it later this week

[calebcartwright]

Just adding a note that ServiceTester which is also imported in services/index.js also imports some dev deps (icedfrisby and icedfrisby-nock) so I'm going to move both ServiceTester and create-service-tester from services/index.js to services/tester.js

[calebcartwright]

As an FYI, our internal Shields deployment is now on a weekly schedule to build the latest Docker image and deploy. We definitely still need #2816 but that will provide some marginal help in this space