Skip to content

Commit

Permalink
chore: remove compile protos from post processor (#1899)
Browse files Browse the repository at this point in the history
* chore: remove compile protos from post processor
  • Loading branch information
sofisl authored Nov 15, 2023
1 parent d7b3591 commit 7324804
Show file tree
Hide file tree
Showing 5 changed files with 2 additions and 97 deletions.
5 changes: 2 additions & 3 deletions docker/owlbot/nodejs_mono_repo/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ RUN find /synthtool -type d -exec chmod a+x {} \;

# Install dependencies used for post processing:
# * gts/typescript are used for linting.
# * google-gax and gapic-tools are used for compiling protos.
RUN cd /synthtool && mkdir node_modules && npm i [email protected] [email protected] \
[email protected] @google-cloud/[email protected] [email protected]
RUN cd /synthtool && mkdir node_modules && npm i [email protected] \
[email protected] @google-cloud/[email protected]

ENTRYPOINT [ "/bin/bash", "/synthtool/docker/owlbot/nodejs_mono_repo/entrypoint.sh" ]
27 changes: 0 additions & 27 deletions synthtool/languages/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,43 +234,16 @@ def fix_hermetic(hide_output=False):
)


def compile_protos(hide_output=False):
"""
Compiles protos into .json, .js, and .d.ts files using
compileProtos script from google-gax.
"""
logger.debug("Compiling protos...")
shell.run(["npx", "compileProtos", "src"], hide_output=hide_output)


# TODO: delete these functions if it turns out we no longer
# need them to be hermetic.
def compile_protos_hermetic(hide_output=False):
"""
Compiles protos into .json, .js, and .d.ts files using
compileProtos script from google-gax. Assumes that compileProtos
is already installed in a well known location on disk (node_modules/.bin).
"""
logger.debug("Compiling protos...")
shell.run(
["node_modules/.bin/compileProtos", "src"],
check=True,
hide_output=hide_output,
)


def postprocess_gapic_library(hide_output=False):
logger.debug("Post-processing GAPIC library...")
install(hide_output=hide_output)
fix(hide_output=hide_output)
compile_protos(hide_output=hide_output)
logger.debug("Post-processing completed")


def postprocess_gapic_library_hermetic(hide_output=False):
logger.debug("Post-processing GAPIC library...")
fix(hide_output=hide_output)
compile_protos(hide_output=hide_output)
logger.debug("Post-processing completed")


Expand Down
36 changes: 0 additions & 36 deletions synthtool/languages/node_mono_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,52 +335,16 @@ def fix_hermetic(relative_dir, hide_output=False):
)


def compile_protos(hide_output=False, is_esm=False):
"""
Compiles protos into .json, .js, and .d.ts files using
compileProtos script from google-gax.
"""
logger.debug("Compiling protos...")
command = (
["npx", "compileProtos", "src"]
if not is_esm
else ["npx", "compileProtos", "esm/src", "--esm"]
)
shell.run(command, hide_output=hide_output)


def compile_protos_hermetic(relative_dir, is_esm=False, hide_output=False):
"""
Compiles protos into .json, .js, and .d.ts files using
compileProtos script from google-gax. Assumes that compileProtos
is already installed in a well known location on disk (node_modules/.bin).
"""
logger.debug("Compiling protos...")
command = (
[f"{_TOOLS_DIRECTORY}/node_modules/.bin/compileProtos", "esm/src", "--esm"]
if not is_esm
else [f"{_TOOLS_DIRECTORY}/node_modules/.bin/compileProtos", "esm/src", "--esm"]
)
shell.run(
command,
cwd=relative_dir,
check=True,
hide_output=hide_output,
)


def postprocess_gapic_library(hide_output=False, is_esm=False):
logger.debug("Post-processing GAPIC library...")
install(hide_output=hide_output)
fix(hide_output=hide_output)
compile_protos(hide_output=hide_output, is_esm=is_esm)
logger.debug("Post-processing completed")


def postprocess_gapic_library_hermetic(relative_dir, hide_output=False, is_esm=False):
logger.debug("Post-processing GAPIC library...")
fix_hermetic(relative_dir, hide_output=hide_output)
compile_protos_hermetic(relative_dir, hide_output=hide_output, is_esm=is_esm)
logger.debug("Post-processing completed")


Expand Down
7 changes: 0 additions & 7 deletions tests/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,19 +169,12 @@ def test_fix(self, shell_run_mock):
calls = shell_run_mock.call_args_list
assert any(["npm run fix" in " ".join(call[0][0]) for call in calls])

@patch("synthtool.shell.run")
def test_compile_protos(self, shell_run_mock):
node.compile_protos()
calls = shell_run_mock.call_args_list
assert any(["npx compileProtos src" in " ".join(call[0][0]) for call in calls])

@patch("synthtool.shell.run")
def test_postprocess_gapic_library(self, shell_run_mock):
node.postprocess_gapic_library()
calls = shell_run_mock.call_args_list
assert any(["npm install" in " ".join(call[0][0]) for call in calls])
assert any(["npm run fix" in " ".join(call[0][0]) for call in calls])
assert any(["npx compileProtos src" in " ".join(call[0][0]) for call in calls])


# postprocess_gapic_library_hermetic() must be mocked because it depends on node modules
Expand Down
24 changes: 0 additions & 24 deletions tests/test_node_mono_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,43 +354,19 @@ def test_fix(self, shell_run_mock):
calls = shell_run_mock.call_args_list
assert any(["npm run fix" in " ".join(call[0][0]) for call in calls])

@patch("synthtool.shell.run")
def test_compile_protos(self, shell_run_mock):
node_mono_repo.compile_protos()
calls = shell_run_mock.call_args_list
assert any(["npx compileProtos src" in " ".join(call[0][0]) for call in calls])

@patch("synthtool.shell.run")
def test_compile_protos_esm(self, shell_run_mock):
node_mono_repo.compile_protos(is_esm=True)
calls = shell_run_mock.call_args_list
assert any(
[
"npx compileProtos esm/src --esm" in " ".join(call[0][0])
for call in calls
]
)

@patch("synthtool.shell.run")
def test_postprocess_gapic_library(self, shell_run_mock):
node_mono_repo.postprocess_gapic_library()
calls = shell_run_mock.call_args_list
assert any(["npm install" in " ".join(call[0][0]) for call in calls])
assert any(["npm run fix" in " ".join(call[0][0]) for call in calls])
assert any(["npx compileProtos src" in " ".join(call[0][0]) for call in calls])

@patch("synthtool.shell.run")
def test_postprocess_gapic_library_esm(self, shell_run_mock):
node_mono_repo.postprocess_gapic_library(is_esm=True)
calls = shell_run_mock.call_args_list
assert any(["npm install" in " ".join(call[0][0]) for call in calls])
assert any(["npm run fix" in " ".join(call[0][0]) for call in calls])
assert any(
[
"npx compileProtos esm/src --esm" in " ".join(call[0][0])
for call in calls
]
)


# postprocess_gapic_library_hermetic() must be mocked because it depends on node modules
Expand Down

0 comments on commit 7324804

Please sign in to comment.