Fetch docker names without github token

[lrlunin]

This fix in combination with #537 allows user to send pull requests from forks and all workflows be executed. The issue before was the fact the the GH REST API request required the secret token for fetch of the docker images list.

Now this is done matching the filenames in docker directory.

[github-actions]

📚 Documentation

📁 Download as zip
🔍 View online

[github-actions]

Coverage Report

File	Stmts	Miss	Cover	Missing
src/mrpro/algorithms/csm
inati.py	24	1	96%	44
walsh.py	16	1	94%	34
src/mrpro/algorithms/dcf
dcf_voronoi.py	53	4	92%	15, 48–49, 76
src/mrpro/algorithms/optimizers
adam.py	20	1	95%	69
src/mrpro/algorithms/reconstruction
DirectReconstruction.py	28	16	43%	51–71, 85
IterativeSENSEReconstruction.py	13	1	92%	76
Reconstruction.py	50	22	56%	42, 54–56, 80–87, 104–113
RegularizedIterativeSENSEReconstruction.py	41	17	59%	96–100, 114–139
src/mrpro/data
AcqInfo.py	128	3	98%	26, 169, 207
CsmData.py	29	3	90%	15, 82–84
DcfData.py	45	8	82%	18, 66, 78–83
IData.py	67	9	87%	119, 125, 129, 159–167
IHeader.py	75	7	91%	75, 109, 127–131
KHeader.py	153	17	89%	25, 119–123, 150, 199, 210, 217–218, 221, 228, 260–271
KNoise.py	31	15	52%	39–52, 56–61
KTrajectory.py	69	5	93%	178–182
MoveDataMixin.py	140	18	87%	15, 113, 129, 143–145, 207, 323–325, 338, 417, 437–438, 440, 455–456, 458
QData.py	39	7	82%	42, 65–73
Rotation.py	674	35	95%	100, 198, 335, 433, 477, 495, 581, 583, 592, 626, 628, 691, 768, 773, 776, 791, 808, 813, 889, 1077, 1082, 1085, 1109, 1113, 1240, 1242, 1250–1251, 1315, 1397, 1690, 1846, 1881, 1885, 1996
SpatialDimension.py	232	21	91%	34, 104, 141, 148, 154, 274–276, 289–291, 325, 343, 356, 369, 382, 395, 404–405, 420, 429
acq_filters.py	12	1	92%	47
src/mrpro/data/_kdata
KData.py	134	18	87%	109–110, 125, 132, 142, 150, 204–205, 243, 248–249, 268–279
KDataRemoveOsMixin.py	29	2	93%	44, 46
KDataSelectMixin.py	19	2	89%	48, 63
KDataSplitMixin.py	48	3	94%	53, 84, 93
src/mrpro/data/traj_calculators
KTrajectoryCalculator.py	25	2	92%	23, 45
KTrajectoryIsmrmrd.py	13	2	85%	41, 50
KTrajectoryPulseq.py	31	1	97%	54
src/mrpro/operators
CartesianSamplingOp.py	89	3	97%	118, 157, 280
ConstraintsOp.py	60	2	97%	46, 48
EndomorphOperator.py	65	2	97%	228, 234
FiniteDifferenceOp.py	27	2	93%	40, 105
FourierOp.py	158	3	98%	263, 381, 386
Functional.py	71	5	93%	20–22, 117, 119
GridSamplingOp.py	136	9	93%	72–73, 82–83, 90–91, 94, 96, 98
LinearOperator.py	168	10	94%	55, 91, 190, 220, 261, 270, 278, 287, 295, 320
LinearOperatorMatrix.py	158	16	90%	82, 119, 152, 161, 166, 175–178, 191–194, 203, 215, 304, 331, 359
MultiIdentityOp.py	13	2	85%	43, 48
Operator.py	78	2	97%	25, 74
ProximableFunctionalSeparableSum.py	39	3	92%	50, 103, 110
SliceProjectionOp.py	173	8	95%	44, 61, 63, 69, 206, 227, 260, 300
WaveletOp.py	120	5	96%	152, 170, 205, 210, 233
ZeroPadOp.py	16	1	94%	30
src/mrpro/utils
filters.py	62	2	97%	44, 49
reshape.py	60	1	98%	191
slice_profiles.py	46	6	87%	20, 36, 113–116, 149
sliding_window.py	34	1	97%	34
split_idx.py	10	2	80%	43, 47
summarize_tensorvalues.py	11	9	18%	20–29
typing.py	18	11	39%	8–23
zero_pad_or_crop.py	31	6	81%	26, 30, 54, 57, 60, 63
TOTAL	4912	353	93%

Tests	Skipped	Failures	Errors	Time
2262	0 💤	0 ❌	0 🔥	2m 7s ⏱️

[fzimmermann89]

would it be easy to make the workflow fail if no docker files were found? just as a small safety net

[lrlunin]

would it be easy to make the workflow fail if no docker files were found? just as a small safety net

As my first commit experience tells - it will fail anyway in the next step when it will try to fetch the docker. But I'll take a look now.

[ckolbPTB]

Do we also have to adapt the docker-workflow?

[lrlunin]

Do we also have to adapt the docker-workflow?

I do not think so. This will not be executed at pull-request. It interacts with registry and tokens only on github.event_name == 'push' i.e. push on main.

[ckolbPTB]

It should be triggered if anything related to the docker image (toml file or the docker file itself) changes, shouldn't it?

[fzimmermann89]

pushing the test docker images cannot work from forks.
so it can realistically only be done here anyways. same with releases to testpypi.

so release and changes to docker will always be done on the main repo.

[fzimmermann89]

we coulld change the docker action to also use sed (or change it here to also use jq), as on both pretty much the same step is done to obtain the imagenames, but with slightly different commands..

[lrlunin]

we coulld change the docker action to also use sed (or change it here to also use jq), as on both pretty much the same step is done to obtain the imagenames, but with slightly different commands..

The sed here is not really replaceable here with jq or jq in later steps is replaceable with sed. For the $GITHUB_OUTPUT we need a JSON like array ["one", "two", "three"] while the first step in the pytest.yml is done purely with the bash arrays. The reason we needed the jq before was the JSON answer from the GH API request.

I would change the cd ... & ls ... with the find ... in the docker.yml and docs.yml.

[ckolbPTB]

so release and changes to docker will always be done on the main repo.

would be good to discuss then in a meeting how we envisage the new workflow

[ckolbPTB]

Quick question: If I create a fork and then a PR to main. The PR gets approved and I merge the PR into main. Currently this would trigger our docker-pipeline as "push to main". This will run on main and hence the github token should be available, shouldn't it?

[lrlunin]

Quick question: If I create a fork and then a PR to main. The PR gets approved and I merge the PR into main. Currently this would trigger our docker-pipeline as "push to main". This will run on main and hence the github token should be available, shouldn't it?

I have expected it to do so. Only after push on branch main the push to registry will be initiated and the only this really needs to have a token for push.

I also think that right now there is a special token generated and placed in secrets due to that API request. After this pull-request It can be deleted if we use a default GITHUB_TOKEN with given "write permissions" for workflows to push.

[fzimmermann89]

I am looking forward to getting this PR merged -- and then trying it out.

I don't think there is a good way to test if this really fixes all issues with forks. It looks like it should work ;)

[lrlunin]

I am looking forward to getting this PR merged -- and then trying it out.

I don't think there is a good way to test if this really fixes all issues with forks. It looks like it should work ;)

I can do it with my "mock" repo or you can just create an own if you wish.

Or you can run the workflow locally with the act with env push main and then check this out ;-)
I'll try this today evening.