Introduce Dockerfile support to maestro workflow #68
Conversation
build.sh
Outdated
| build_built=true | ||
| fi | ||
|
|
||
| if [ -f project.toml ]; then |
There was a problem hiding this comment.
This will run together with the 35 line, if any project has Dockerfile (to local development) and Project.toml to build.
There was a problem hiding this comment.
@fagianijunior having them both is definitely not ideal but how to handle that? which one takes precedence? I dislike the idea of adding yet another ENV for defining it as proposed in the last commit.
There was a problem hiding this comment.
@MatheusdeMelo Please take a look at the comments in order to fix and improve this PR
build.sh
Outdated
| cd $MAESTRO_BUILD_FOLDER | ||
| fi | ||
|
|
||
| if [ $MAESTRO_BUILD_MODE == Dockerfile ]; then |
There was a problem hiding this comment.
this check differs from the one in the elif...
if you have changed the directory why don't just check for the file presence as before?
build.sh
Outdated
| exit 0 | ||
| fi | ||
|
|
||
| if [ ! -z $MAESTRO_BUILD_FOLDER ]; then |
There was a problem hiding this comment.
I'd suggest $WORKLOAD_PATH instead of $MAESTRO_BUILD_FOLDER
build.sh
Outdated
| fi | ||
|
|
||
| if [ ! -z $MAESTRO_BUILD_FOLDER ]; then | ||
| cd $MAESTRO_BUILD_FOLDER |
There was a problem hiding this comment.
Maybe instead of changing directory you could leverage the docker and pack path parameters
MatheusdeMelo
force-pushed
the
feature/add-dockerfile-support
branch
from
d678863
to
1181d51
Compare
|
@fagiani, if a person want's to use buildpack and the repository is |
…estro into feature/add-dockerfile-support
| if [ -z "$WORKLOAD_PATH" ]; then | ||
| $WORKLOAD_PATH=. |
There was a problem hiding this comment.
@fagiani here $WORKLOAD_PATH receive the "." as value if it's empty, thinking about this with @fagianijunior we think wich is unnecessary utilize bash parameter expansion to change the value to "." again.
README.md
Outdated
| @@ -79,6 +79,7 @@ Variable | Description | Examples/Values | Default | |||
| `DEPLOYMENT_CIRCUIT_BREAKER_RULE` | Enable or disable circuit breaker | `enable=true,rollback=true` | |||
| `ECS_CONTAINER_STOP_TIMEOUT` | Set stopTimeout on taskdefinition | min: 0, max: 120, default: 30 | |||
| `TZ`| Set this variable to the desired task timezone | America/Sao_Paulo | |||
| `WORKLOAD_PATH`| Set the workload path to run build commands on correct directory| ./Projects/my-app, ./ | |||
There was a problem hiding this comment.
"Set the workload path to run build commands on correct directory" should do it
There was a problem hiding this comment.
Changed to new description.
|
@fagiani @fagianijunior this PR can be merged or is necessary other changes to merge? |
This PR aims to add the possibility to use
docker buildas an alternative topack buildin the maestro workflow.Currently buildpacks will rely on the
.envfile to feed environment variables during the build and we need to figure if that is required for this new approach and if so, how.@fagianijunior Please start tests with a sample project and create additional commits as required.