search

jupyterhub / chartpress

automate building and publishing images for helm charts
BSD 3-Clause "New" or "Revised" License
55 stars 24 forks source link

PR Discussion - Manage uncommitted changes #49

Open consideRatio opened 4 years ago

consideRatio commented 4 years ago

If chartpress is run with unstaged changes that makes us require a image rebuild, what should we do?

Currently, the process chartpress works like...

  1. Get the last commit where dependent files for an image where changed.
  2. See if that image is available locally or at DockerHub and build it if it isn't.

So, if we have unstaged changes, chartpress will not rebuild or update the helm chart and images. I think it could be good to do this by default, but that the user may want to understand if this happens by being warned they need to commit those changes for chartpress to care.

I propose we add a check for unstaged changes of relevance and print a warning message about this. Oh no wait... chartpress could mess up badly unless we fail sometimes... Consider if it realize it requires a rebuild, but there are more unstaged changes of relevance for the image build. Then it could end up building a image for a commit tag that isn't representing the state of that commit.

We have to decide how we want chartpress to behave in at least two situations.

There are unstaged changes influencing an image, and...

  1. the tag is not available and needs to be rebuilt
  2. the tag is already available and doesn't need to be rebuilt

I think we should fail hard for 1), and for 2) the user at least needs to be informed.

Technically, to conclude if there are unstaged changes, we could use the currently unused (since 55fe935) function path_touched where we would pass the commit range of HEAD for it to catch staged and unstaged changes that has not yet been committed.

minrk commented 4 years ago

I think checking for uncommitted changes is great. I would try to avoid doing things too strict or complicated, and just append ".dirty" to versions if there are uncommitted changes in the inputs, probably also omitting a warning. I don't think failing hard is what we should do.

minrk commented 4 years ago

<pedant>I think it should be uncommitted changes we need to deal with. Staged or not shouldn't be relevant.</pedant>