Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improved nodejs codegen build and fixed race condition with update_ge… #117

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jameslinjl
Copy link
Contributor

@jameslinjl jameslinjl commented Jan 5, 2023

  • brought the nodejs codegen build up to par with the golang codegen build
  • in testing the nodejs codegen, I actually happened upon a bug in the golang implementation. docker run -d backgrounds the entire docker container, which means that there is no guarantee that the code gen has completed prior to attempting the subsequent docker cp command. In addition, the container doesn't stay alive unless its foreground process stays alive. To solve for these race conditions, we override the container's original foreground process with tail -f > /dev/null so that it will be around until we kill it and then exec the code gen command synchronously prior to cping it. With both the golang and the python bindings, I never ran into this issue because the codegen was performant enough not to run into the race condition (not so for nodejs 😉 )

@jameslinjl
Copy link
Contributor Author

@bdferris-v2 sorry for the rapid pings, but I found a fun race condition in the implementation of update_generated_code.sh. not an emergency by any means, but would be good to patch up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant