Allow specifying multiple clients for run_examples #1148
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jul-sh
force-pushed
the
run_examples_run_all_clients
branch
from
7fbfa48 to
92f52d1
Compare
ipetr0v
approved these changes
Jun 12, 2020
| --root-tls-certificate="${SCRIPTS_DIR}/../examples/certs/local/ca.pem" \ | ||
| "${CLIENT_ARGS[@]-}" | ||
| else | ||
| echo "The ${EXAMPLE} example does not contain a ${client_language} client. Skipping this client." |
There was a problem hiding this comment.
IIUC If we will run -c rust on the example that doesn't support it, there will be no error, and the example script will return 0 as a status.
There was a problem hiding this comment.
Yes, that is indeed correct. It will print a warning, but it won't fail.
If we think it's important to fail in this case, we'd need to implement a slightly different approach, such as -c all to run all available clients.
What do you think?
There was a problem hiding this comment.
I don't think the checks for which client languages exist are working properly, and the fail-open nature of the checks means that our Cloudbuild is currently running almost none of the examples:
% grep "example does not contain" log-bdcc0238-2511-4076-90da-4810b7719888.txt | grep -v ': +'
Step #9 - "run_examples": The abitest example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The abitest example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The abitest example does not contain a nodejs client. Skipping this client.
Step #9 - "run_examples": The translator example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The translator example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The translator example does not contain a nodejs client. Skipping this client.
Step #9 - "run_examples": The trusted_information_retrieval example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The trusted_information_retrieval example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The trusted_information_retrieval example does not contain a nodejs client. Skipping this client.
Step #9 - "run_examples": The hello_world example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The hello_world example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The chat example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The chat example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The chat example does not contain a nodejs client. Skipping this client.
Step #9 - "run_examples": The running_average example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The running_average example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The running_average example does not contain a nodejs client. Skipping this client.
Step #9 - "run_examples": The private_set_intersection example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The private_set_intersection example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The private_set_intersection example does not contain a nodejs client. Skipping this client.
Step #9 - "run_examples": The machine_learning example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The machine_learning example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The machine_learning example does not contain a nodejs client. Skipping this client.
Step #9 - "run_examples": The aggregator example does not contain a rust client. Skipping this client.
Step #9 - "run_examples": The aggregator example does not contain a cpp client. Skipping this client.
Step #9 - "run_examples": The aggregator example does not contain a nodejs client. Skipping this client.
Step #10 - "run_examples_cpp": The hello_world example does not contain a cpp client. Skipping this client.
Step #10 - "run_examples_cpp": The tensorflow example does not contain a cpp client. Skipping this client.@juliettepretot please could you take a look?
| @@ -38,12 +46,12 @@ done | |||
| examples="$(find examples -mindepth 2 -maxdepth 4 -type d -regex '.*/module.*/'"${application_language}"'$' | cut -d'/' -f2 | uniq)" | |||
| for example in ${examples}; do | |||
| if [[ "${example}" == 'aggregator' ]]; then | |||
| "${SCRIPTS_DIR}/run_example" -s "${server}" -a "${application_language}" -e aggregator -- --bucket=test --data=1:10,2:20,3:30 | |||
| "${SCRIPTS_DIR}/run_example" -s "${server}" -a "${application_language}" -c "${client_languages}" -e aggregator -- --bucket=test --data=1:10,2:20,3:30 | |||
There was a problem hiding this comment.
I think at this point we might have to implement all lang clients for all examples, since example folders are often exhaustive - so developers will have less time figuring out how to work with Oak.
| @@ -118,7 +100,7 @@ steps: | |||
| # Run clang-tidy after the examples finish running, since they share the same `output_base`. | |||
| - name: 'gcr.io/oak-ci/oak:latest' | |||
| id: run_clang_tidy | |||
| waitFor: ['run_example_hello_world_nodejs'] | |||
| waitFor: ['run_examples_cpp'] | |||
There was a problem hiding this comment.
It may be called run_examples_with_cpp_app, since it may be confusing what cpp stands for.
|
Reproducibility Index: Reproducibility Index diff: |
This was referenced Jun 18, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adresses #1145
Checklist
Cloudbuild
cover any TODOs and/or unfinished work.
construction.