diff mbox

[v3,01/16] docker: Don't force rebuilds

Message ID 87d1hej87v.fsf@possimpible.ozlabs.ibm.com
State Rejected
Headers show

Commit Message

Daniel Axtens Nov. 29, 2016, 9:18 p.m. UTC 
Hi Stephen,

> It's frustrating to have to rebuild the Docker image everytime the
> requirements change. Simply warn the user instead and let them take the
> intiative where necessary.

I'm worried that this will cause some quirky, difficult to debug and
difficult to reproduce issues. Having said that, I do appreciate that
having to rebuild is annoying, and that you do it more than I do!

I have this completely untested suggestion:


This will force you to explicitly override the requirements. My hope is
that requiring this more active step to do something dangerous will make
you more likely to remember that you're in the danger zone and thus
remember to do a build when convenient.

Also this probably doesn't need to be part of this series.

Regards,
Daniel


>
> Signed-off-by: Stephen Finucane <stephen@that.guru>
> ---
>  tools/docker/entrypoint.sh | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/tools/docker/entrypoint.sh b/tools/docker/entrypoint.sh
> index 5a23fa3..d06b271 100755
> --- a/tools/docker/entrypoint.sh
> +++ b/tools/docker/entrypoint.sh
> @@ -40,9 +40,8 @@ fi
>  for x in /tmp/requirements-*.txt; do
>      if ! cmp $x ~/patchwork/$(basename $x); then
>          echo "A requirements file has changed."
> -        echo "Please rebuild the patchwork image:"
> +        echo "You may need to rebuild this image:"
>          echo "    docker-compose build web"
> -        exit 1
>      fi
>  done
>  
> -- 
> 2.7.4
>
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork

Comments

Stephen Finucane Jan. 2, 2017, 8:13 p.m. UTC | #1 
On Wed, 2016-11-30 at 08:18 +1100, Daniel Axtens wrote:
> Hi Stephen,
> 
> > It's frustrating to have to rebuild the Docker image everytime the
> > requirements change. Simply warn the user instead and let them take
> > the
> > intiative where necessary.
> 
> I'm worried that this will cause some quirky, difficult to debug and
> difficult to reproduce issues. Having said that, I do appreciate that
> having to rebuild is annoying, and that you do it more than I do!

Hmm, I get where you're coming from but this is no different to what
would happen were you running on the local machine. We usually bump
and/or add additional requirements as we need them and the application
will simply fail if there are any issues with these requirements. Given
that we still give a warning if there are changes, the user should be
able to identify these issues. I can take the approach suggested below
if you insist, but it's a good deal more code and I don't know if
there's any benefit. Also, it's an extra parameter I'd have to type
every time :)

Stephen

> I have this completely untested suggestion:
> 
> diff --git a/tools/docker/entrypoint.sh b/tools/docker/entrypoint.sh
> index 5a23fa36b9cb..da48608b2b2b 100755
> --- a/tools/docker/entrypoint.sh
> +++ b/tools/docker/entrypoint.sh
> @@ -37,12 +37,22 @@ if [ ! -f
> ~patchwork/patchwork/tools/docker/entrypoint.sh ]; then
>  fi
>  
>  # check if we need to rebuild because requirements changed
> +# allow the user to override with "--force-requirements"
> +FORCE_REQUIREMENTS=0
> +if [[ "$1" == "--force-requirements" ]]; then
> +    shift
> +    FORCE_REQUIREMENTS=1
> +fi
>  for x in /tmp/requirements-*.txt; do
>      if ! cmp $x ~/patchwork/$(basename $x); then
>          echo "A requirements file has changed."
>          echo "Please rebuild the patchwork image:"
>          echo "    docker-compose build web"
> -        exit 1
> +       if [[ "$FORCE_REQUIREMENTS" == 1 ]]; then
> +           echo "WARNING: Proceeding anyway as requested."
> +       else
> +            exit 1
> +       fi
>      fi
>  done
> 
> 
> This will force you to explicitly override the requirements. My hope
> is
> that requiring this more active step to do something dangerous will
> make
> you more likely to remember that you're in the danger zone and thus
> remember to do a build when convenient.
> 
> Also this probably doesn't need to be part of this series.
>
> Regards,
> Daniel
diff mbox

Patch

diff --git a/tools/docker/entrypoint.sh b/tools/docker/entrypoint.sh
index 5a23fa36b9cb..da48608b2b2b 100755
--- a/tools/docker/entrypoint.sh
+++ b/tools/docker/entrypoint.sh
@@ -37,12 +37,22 @@  if [ ! -f ~patchwork/patchwork/tools/docker/entrypoint.sh ]; then
 fi
 
 # check if we need to rebuild because requirements changed
+# allow the user to override with "--force-requirements"
+FORCE_REQUIREMENTS=0
+if [[ "$1" == "--force-requirements" ]]; then
+    shift
+    FORCE_REQUIREMENTS=1
+fi
 for x in /tmp/requirements-*.txt; do
     if ! cmp $x ~/patchwork/$(basename $x); then
         echo "A requirements file has changed."
         echo "Please rebuild the patchwork image:"
         echo "    docker-compose build web"
-        exit 1
+       if [[ "$FORCE_REQUIREMENTS" == 1 ]]; then
+           echo "WARNING: Proceeding anyway as requested."
+       else
+            exit 1
+       fi
     fi
 done