Multiple Hooks simultaneously

[brownbaerchen]

You can now add hooks either as a single hook, so the API is backwards compatible, or as an array of hooks. All hooks will be called one after another in the order that they are added whenever previously a single hook was called. The order should not matter since the hooks are not supposed to modify anything.
I wrote a few hooks that record small pieces of data that is interesting when using a certain convergence controller, which are added automagically when the respective convergence controller is added. For instance after each step, the step size is recorded as 'dt' when using adaptivity and so on.

Since the hooks have no access to the controller, I left the __stats variable in the hooks and merge them after the fact in the controller.

Since I need the __num_restarts variable for add_to_stats such that I can filter recomputed values afterwards, it is unfortunately still necessary to call the parent method with super().whatever() just like before.
However, for most people it doesn't matter. Actually, my tests pass without calling the parent's methods, but there is a number of fringe cases where it might make a difference. So whenever we write general hooks, we should keep calling the parent's method, but individual projects can choose to forgo this if they never restart anything.

Closes #243

[brownbaerchen]

Why is mirror_to_gitlab not working? It works now on my fork, but not here. Jakob mentioned that you should use on: pull_request_target instead of on: pull_request here, I think.
This is also what he writes in the readme now.
No idea, if this is what causes the problem here, though.

[jakob-fritz]

This is exactly, what causes the problem.
With "on: pull_request" the secrets from the source are used, whereas with "on: pull_request_target" the secrets from target are used.

This was missing in the first version of the readme and is now added as brownbaerchen mentioned.

[pancetta]

@brownbaerchen Can you change that in the PR, please?

[brownbaerchen]

@jakob-fritz Now the pipeline is not executed when I start a PR. What did I do wrong?

[jakob-fritz]

I don' know, why it is not shown. When I look at the commit itself I can see a GitHub-Workflow that started

I have no idea, why this is not shown in the merge-request

[brownbaerchen]

I don' know, why it is not shown. When I look at the commit itself I can see a GitHub-Workflow that started

I have no idea, why this is not shown in the merge-request

No that's different. That is run on my fork on push. But since I can do whatever on my fork, we need to run it here as well. The actions tab does not show any action run for the latest commit on this PR on the main repo.

[jakob-fritz]

Then, maybe @pancetta needs to confirm this execution first? As the secrets (and potentially the compute-time in other cases) from his repo are used?

[pancetta]

Can't do. Nothing to confirm here!

[brownbaerchen]

People seem to have some security concerns regarding pull_request_target. And it makes sense that enabling anybody to run anything on KIT machines is not what we want to do here. As #194 demonstrates, this is actually a valid concern. Before we merge something like this into master, we should consider opening this only to known collaborators, if that's an option. Maybe the gitlab stuff should be skipped if a first time collaborator opens a PR with Robert having to start the gitlab stuff manually?

This actually raises the following point: If anyone can change the pipeline to pull_request_target, than anyone can just enable themselves to download pictures of famous people to KIT machines. Is this not working because of safety reasons? Does Robert have to change to on: pull_request_target in the master branch first?

[jakob-fritz]

People seem to have some security concerns regarding pull_request_target. And it makes sense that enabling anybody to run anything on KIT machines is not what we want to do here. As #194 demonstrates, this is actually a valid concern. Before we merge something like this into master, we should consider opening this only to known collaborators, if that's an option. Maybe the gitlab stuff should be skipped if a first time collaborator opens a PR with Robert having to start the gitlab stuff manually?

This actually raises the following point: If anyone can change the pipeline to pull_request_target, than anyone can just enable themselves to download pictures of famous people to KIT machines. Is this not working because of safety reasons? Does Robert have to change to on: pull_request_target in the master branch first?

That totally makes sense to be sceptic on that. In the documentation I read, that first-time contributors need to be confirmed to the workflow to run in pull-requests (this is why I was wondering if he has to confirm it). This is even more important when using other services (e.g. Gitlab on KIT-machines) and after seeing #194 (no words on that).

So your point is valid, that it makes sense that "pull_request" can only be changed to "pull_request_target" from within the main repo and not from the outside.
If you do not replace it but add it, does might the pipeline then run again (with an error of not accessing the secrets from the main repo)? This might be an indicator, that the "_target"-part is unused until part of the main repo.

[pancetta]

That totally makes sense to be sceptic on that. In the documentation I read, that first-time contributors need to be confirmed to the workflow to run in pull-requests (this is why I was wondering if he has to confirm it).

And I did confirm that @brownbaerchen can run stuff during his PRs. I think it is the duty of the maintainer to take care of PR's content, i.e. if a PR from a first-time contributor contains crap, then get rid of it. I cannot prevent a known contributor from going rogue, though.

[pancetta]

You may want to merge the changes from the master branch, too.

[pancetta]

I think I like this one!