Backend - Fixed handling of sample compilation failure

[Ark-kun]

Fixes #354

---

This change is 

[qimingj]

There is a build-image failure...

[Ark-kun]

@qimingj Looks like the dsl-compiler fails to load the file in some cases. Can you please investigate this?

I suspect that there are some problems with file path handling.
The paths are most likely in the following format:

./xgboost-spark/xgboost-training-cm.py

build-api-server-image:	Step 21/32 : RUN  for pipeline in "$(find . -maxdepth 2 -name '*.py' -type f)"; do dsl-compile --py "$pipeline" --output "$pipeline.tar.gz"; done
build-api-server-image:	 ---> Running in 1af2bec754d0
build-api-server-image:	Traceback (most recent call last):
build-api-server-image:	  File "/usr/local/bin/dsl-compile", line 11, in <module>
build-api-server-image:	    sys.exit(main())
build-api-server-image:	  File "/usr/local/lib/python3.5/site-packages/kfp/compiler/main.py", line 100, in main
build-api-server-image:	    compile_pyfile(args.py, args.function, args.output)
build-api-server-image:	  File "/usr/local/lib/python3.5/site-packages/kfp/compiler/main.py", line 88, in compile_pyfile
build-api-server-image:	    __import__(os.path.splitext(filename)[0])
build-api-server-image:	ImportError: No module named 'xgboost-training-cm'
build-api-server-image:	The command '/bin/sh -c for pipeline in "$(find . -maxdepth 2 -name '*.py' -type f)"; do dsl-compile --py "$pipeline" --output "$pipeline.tar.gz"; done' returned a non-zero code: 1

[qimingj]

You don't need the quotes around find command.

RUN for pipeline in "$(find . -maxdepth 2 -name '*.py' -type f)"; do dsl-compile --py "$pipeline" --output "$pipeline.tar.gz"; done

should be:

RUN for pipeline in $(find . -maxdepth 2 -name '*.py' -type f); do dsl-compile --py "$pipeline" --output "$pipeline.tar.gz"; done

With the quotes the results are treated as a single string with multiple lines.

[Ark-kun]

@qimingj Thanks for fixing the problem. I checked the code locally and it did not occur to me that it would work differently in Docker. I think we should modify the compiler so that it checks the existence of a file and give a meaningful error if the file does not exist.

You don't need the quotes around find command.

I've checked that the quotes are needed when the files can have spaces. Otherwise for breaks them into parts. I wonder why this works differently when run by Docker.

[qimingj]

/lgtm

[qimingj]

The other pair of quotes ("$pipeline") should take care of the space in file names?

I can reproduce the problem locally with your previous command.

[Ark-kun]

The other pair of quotes ("$pipeline") should take care of the space in file names?

Those quotes cannot help when the file name is already split into parts by for:

$ ls -la
total 16
drwxr-xr-x  2 avolkov primarygroup 4096 Nov 26 18:20  .
drwxrwxrwt 16 root    root         4096 Nov 26 18:20  ..
-rw-r--r--  1 avolkov primarygroup    4 Nov 26 18:20 'aa bb'
-rw-r--r--  1 avolkov primarygroup    4 Nov 26 18:20 'cc dd'

$ for file in "$(find . -type f)"; do echo $file; done
./cc dd ./aa bb

$ for file in $(find . -type f); do echo $file; done
./cc
dd
./aa
bb

[qimingj]

/lgtm

[Ark-kun]

/test presubmit-e2e-test

[Ark-kun]

/approve

[IronPan]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Ark-kun, IronPan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• backend/OWNERS [IronPan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Ark-kun]

/test build-image

[Ark-kun]

/test build-image