fix docker build

[savente93]

Issue addressed

Fixes part of #1326

Explanation

A bit more might be necessary but I suspect that a large part of the reason it is broken is that the docker images seemingly haven't been published since v10.1. This step should fix the docker build tasks so we can publish it, after that I'll have to see if additional work is necessary.

Locally the binder test works when I built the docker image locally. it will just have to be published as well after this is merged.

In addition to fixing the docker I've also reworked the test workflow for the binder. instead of invoking the repo2docker cli which tests a lot less than I thought, we build the images themselves, spin them up and see if we can connect to the jupyter server. I think this is as good as we can reasonably test this for now. This doesn't guarantee that the binder will work since that one will try to pull from dockerhub. If that is also important we could implement a routine test with the latest published one, but I'll leave that for now to avoid scope creep.

General Checklist

• Updated tests or added new tests
• Branch is up to date with main
• Tests & pre-commit hooks pass
• Updated documentation
• Updated changelog.rst

Data/Catalog checklist

• data/catalogs/predefined_catalogs.yml has not been modified.
• None of the old data_catalog.yml files have been changed
• data/changelog.rst has been updated
• new file uses LF line endings (done automatically if you used update_versions.py)
• New file has been tested locally
• Tests have been added using the new file in the test suite

Additional Notes (optional)

Add any additional notes or information that may be helpful.

[savente93]

The binder test flow will probably still fail in this PR because it is trying to take the latest docker image from dockerhub, which is now still 10.1

[deltamarnix]

Can you tell me if the new test that you wrote indeed breaks with the old docker image?
As in, how do you know that this test is more sufficient than the previous one?

[savente93]

well, I tested it with an (intentionally broken) docker image and the run failed, which you can see here, https://github.com/Deltares/hydromt/actions/runs/19859860353 and secondly we actually connect to the jupyter werver after the docker image runs, if it fails to start for any reason we won't be able to reach it, so that part should also be better instead of just making sure the image builds (which is what I suspect the previous one did).

The one thing I can't test for is the fact that this one builds the image locally instead of pulling it from dockerhub which is what the binder service does, but if I do the same we can't test these images before releasing. If you want I can write a workflow that tests this with the image from dockerhub that runs periodically. but I thought that was a bit overkill.

I didn't try with a litteral old image, but from observing the errors on the binder it seems like it's a delivery failure, not an issue with the image (as we haven't published a docker image in a while)

[deltamarnix]

I would suggest using the pixi task to create more reproducibility between local and server pipeline. The rest looks fine to me.

[deltamarnix]

I was expecting that docker-build would be called in the github action, but instead a separate action is used. Consider if you want to make use of the pixi tasks during github actions, because it makes it perhaps easier to maintain and reproduce locally vs on our servers.

So this docker-build task is local only? How does one add DOCKER_TAG and DOCKER_TARGET? If it's with environment variables... you can now use task arguments in pixi: https://pixi.sh/latest/workspace/advanced_tasks/#task-arguments, which is more locally controlled during the pixi step.

docker-build = { cmd = [
"docker",
"build",
"-t",
"deltares/hydromt:{{ docker_tag }}",
"--target={{ docker_target }}",
"--build-arg",
"PIXIENV={{ docker_target }}-latest",
".",
],
args=[
  { "arg"="docker_target", "default"="slim" },
  { "arg"="docker_tag",  "default"="dev"}
] }

call it with pixi run docker-build if you want to use the defaults, or e.g. pixi run docker-build full v1.3.0 if you want more control.

[savente93]

I'll rewrite it to use the pixi tasks

[deltamarnix]

Are you keeping the environment variables?

[deltamarnix]

I didn't try with a litteral old image, but from observing the errors on the binder it seems like it's a delivery failure, not an issue with the image (as we haven't published a docker image in a while)

Then perhaps we can also add pushing binder to our release pipeline?

[savente93]

Then perhaps we can also add pushing binder to our release pipeline?

binder is a pull based system, so I'm not aware of a way to do this

[deltamarnix]

binder is a pull based system, so I'm not aware of a way to do this

I mean pushing the binder docker images to dockerhub on release of hydromt

[savente93]

ohhhh right, we could do that yeah, though I have to find out if you can specify which image gets used by binderhub, I'm not sure of that. I can look into that if you want

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[savente93]

I've tried to look through all the binder docs to find how it gets determined which tag gets used, I've not been able to find a definitive answer. the assumption seems to be that there is only one, but I also haven't been able to confirm or deny that. Given that this is a relatively minor point I'm not going to spend more time on it now, but if you like you can make an issue and I'll happily take a look at it more in depth

[deltamarnix]

Some minor comments on the pixi file. But it looks good to me.

[deltamarnix]

Since the binder docker depends on deltares/hydromt:slim, we can add this dependency here in pixi like so:

Suggested change

	]}
	], "depends-on"=["dockker-build-slim"]}

Then you won't have to add two pixi steps in the github action