Skip to content

Report git hash of each repository #401

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
MakisH opened this issue Nov 19, 2023 · 5 comments
Closed

Report git hash of each repository #401

MakisH opened this issue Nov 19, 2023 · 5 comments

Comments

@MakisH
Copy link
Member

MakisH commented Nov 19, 2023

Related to #400.

The actual git commits of each repository are not reported anywhere.

While just develop is nice and easy when triggering the tests, it does not let me reproduce the tests. How do I know that my commit is there?

Even something dirty, such as git show | head -n 5 for each repository would be nice.

@MakisH MakisH changed the title System tests: Report git hash of each repository Report git hash of each repository Nov 19, 2023
@MakisH MakisH mentioned this issue Nov 21, 2023
5 tasks
@valentin-seitz
Copy link
Contributor

What do you mean by repository?

The corresponding solvers+precice versions are "fixed" in the docker-compose.tutorial.yaml.
The ambiguity of using a branch name can be fixed by providing tags as build_args.
I can see why this might be a bit unhandy to do (one doesnt want to create a lot of tags)

One thing i could do is to place a file there that resolves the git heads of each of the branches used. (This however would mean i have to checkout the

But re-triggering this via the github CLI would probably be difficult as its assumed that git clone -b $REF is a working command for most of the dockerfile. For the openfoam-adapter this looks like. git clone --depth 1 -b ${OPENFOAM_ADAPTER_REF} https://github.com/precice/openfoam-adapter.git

Ofc, we can introduce the notion of only using commits, this would however, mean a little refactor on the dockerfile side as it will probably not work out of the box. (this would probably also solve some caching issues, e.g hitting a stale cache)

@MakisH
Copy link
Member Author

MakisH commented Nov 28, 2023

What do you mean by repository?

Well... Each component.

The corresponding solvers+precice versions are "fixed" in the docker-compose.tutorial.yaml.

I can pass build arguments to each component, including which version to use. It is important for me to know, when looking at arbitrary results, which state of each component was used to generate those results.

One thing i could do is to place a file there that resolves the git heads of each of the branches used. (This however would mean i have to checkout the

Why is something like git show | head -n 5 just after the git clone not enough?

Note that this issue is only about reporting, not about passing all kinds of git references to git clone.

@valentin-seitz
Copy link
Contributor

I can pass build arguments to each component, including which version to use. It is important for me to know, when looking at arbitrary results, which state of each component was used to generate those results.

I totally agree on this. The problem is: The actual commit is only known by the docker container. We treat that as a black box right now. We will just stuff some arguments in and in the end we have a container that will happily execute the ./run.sh. What we could do is execute a git show | head -n 5 in each of the the build stages and try to make the systemtests print that.

Note that this issue is only about reporting, not about passing all kinds of git references to git clone.

kind of, but we also run into the issue of stale caches:
Imagine the following requests to the systemtests:

  1. Run with python systemtests.py --build_args=openfoam-adapter:develop
  2. PR merged on openfoam-adapter:develop with critical changes
  3. Run with python systemtests.py --build_args=openfoam-adapter:develop

Line 3 will probably generate cache hit, because the string develop did not change. So the docker builder is like: why should i rebuild. I already have this cached.

This however is not really behavior we want to see?
So since this issue and the currently undocumented caching issue are related, i think we need one solution to both?

One solution would be to just allow tags or commits to be used in the ref of the component. Which also would solve the problem of undocumented refs.
Another alternative would be to manually invalidate the cache via a workflow triggered on every merge (not my preferred solution).

@MakisH
Copy link
Member Author

MakisH commented Nov 29, 2023

Conclusion from yesterday's discussion: Restricting to commits and tags is fine. It should also make caching behavior clearer.

@MakisH
Copy link
Member Author

MakisH commented Jan 7, 2024

Closed by #410

@MakisH MakisH closed this as completed Jan 7, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants