Skip to content

Install logging JS app#1932

Merged
achamayou merged 15 commits into
microsoft:masterfrom
jumaffre:install_js_app
Nov 27, 2020
Merged

Install logging JS app#1932
achamayou merged 15 commits into
microsoft:masterfrom
jumaffre:install_js_app

Conversation

@jumaffre
Copy link
Copy Markdown
Contributor

The primary goal of this PR is to install the logging JS application so that a test CCF service running a simple application can be run with the sandbox. In other words, it is now possible to install the CCF debian package and run the sandbox.sh script (with no arguments) to get a virtual CCF test service running the logging JS app.

Other changes:

  • Moved logging app from src/apps/ to samples/apps/
  • Moved simplebank app from samples/apps to src/apps
  • test_install.sh script now makes use of sandbox
  • Fixed issue with JS logging app where the /log/public endpoints weren't reachable

@jumaffre jumaffre requested a review from a team as a code owner November 25, 2020 17:58
@ghost
Copy link
Copy Markdown

ghost commented Nov 25, 2020

install_js_app@15817 aka 20201126.18 vs master ewma over 50 builds from 15287 to 15812
images

Comment thread tests/test_install.sh Outdated
"$INSTALL_PREFIX"/bin/sandbox.sh --verbose &

# Wait for service to be open
sleep 45
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems like a long time, but I guess we don't have a convenient file to wait on anymore.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think we should find a better way to wait. We could grep the output of sandbox.sh, or wait for some file to be written by the infra, or have a separate util (in bash or Python) which polls the node until it seems active?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I've added a quick check for the service being open with curl.

Comment thread tests/test_install.sh
source env/bin/activate
python -m pip install -U -r "$INSTALL_PREFIX"/bin/requirements.txt
python -m pip install ../../../python
python ../../../python/tutorial.py ./workspace/sandbox_0/0.ledger/ ./workspace/sandbox_common/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the waiting ought to happen at the top of tutorial, using the client timeout logic we have in Python anyway?

Copy link
Copy Markdown
Contributor Author

@jumaffre jumaffre Nov 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed above, we wait for service opening directly inside test_install.sh (where the sleep ... was). I believe this is neater and keeps the tutorial.py untouched.

Comment thread tests/test_install.sh Outdated
@jumaffre
Copy link
Copy Markdown
Contributor Author

Looks like the daily build failed when testing the install: https://dev.azure.com/MSRC-CCF/CCF/_build/results?buildId=15807&view=logs&j=5435e0ac-25e5-5426-50be-61b0d0ea8d34&t=fe98d859-2537-5570-75af-de67d4290f3a

Investigating this now to find out what the issue is.

Comment thread tests/sandbox/sandbox.sh
done

extra_args=("$@")
if [ ${is_package_specified} == false ] && [ -f "${VERSION_FILE}" ]; then
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes behaviour different for build vs install tree when only --js-app-bundle is given because the extra args here are added after the user args, overriding them and starting the wrong app.

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.

4 participants