Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

CI/Dockerfile: port 1.8 changes to 1.9 #1049

Merged
merged 4 commits into from
Aug 8, 2024

Conversation

pjonsson
Copy link
Contributor

@pjonsson pjonsson commented Aug 7, 2024

Cherry-pick the changes to CI/Dockerfile from 1.8 to 1.9. Cherry-picking commit 9579b4d gives a lot of conflicts, so make a separate small commit that removes the dash in docker-compose which are the necessary changes for this PR.


📚 Documentation preview 📚: https://datacube-ows--1049.org.readthedocs.build/en/1049/

This is the modern way of invoking Docker
compose.
Add a script for switching UID/GID
of the user inside the container.
Use this to avoid making the directories
the CI uses world-writable.
This removes some warning printouts
in the tests.
@pjonsson
Copy link
Contributor Author

pjonsson commented Aug 7, 2024

Edit: first error was a merge-error from me, updated error and added the question about docker-compose.cleandb.yaml.

Edit 2: tests were failing because they were using an image that did not contain the changes in this PR. Added a --build to test.yml to get this through the CI. My thoughts about squashing vs amending/fixups and my question about docker-compose.cleandb.yaml are still relevant.

I don't know much about git, so I generally try to stay in my lane. I'm guessing it was a squash in the CI merge command into what is now commit 9579b4d. I don't think it's possible to avoid pain completely, but in general, I think it would be easier to take the pain and amend/fixup broken commits in pull requests and force-pushing so you get a "clean" PR/commit history, rather than adding commits to the PR and squashing everything when merging. With a reasonable commit hygiene, that strategy would reduce the size of merge conflicts, and that would make it easier to keep the two branches in sync.

Commit 1c22b2c changes the docker-compose.db.yaml file, should the same changes have been made to docker-compose.cleandb.yaml?

I don't see why this PR would make any change to these parts, but the failing CI job cannot find the datacube config:

+ datacube system init
Error: No datacube config found
Error: Process completed with exit code 137.

Do you by any chance happen to know what I might have messed up?

@pjonsson pjonsson force-pushed the ci-tweaks-d19 branch 9 times, most recently from cc9e0ac to fff42e4 Compare August 7, 2024 12:51
Copy link

codecov bot commented Aug 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.27%. Comparing base (722bc6e) to head (705b91c).
Report is 9 commits behind head on develop-1.9.

Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff               @@
##           develop-1.9    #1049      +/-   ##
===============================================
- Coverage        93.77%   93.27%   -0.50%     
===============================================
  Files               46       53       +7     
  Lines             6573     7038     +465     
===============================================
+ Hits              6164     6565     +401     
- Misses             409      473      +64     

see 8 files with indirect coverage changes

Add a health check to compose and
use that in the CI instead of the
wait-for-db-script.
@SpacemanPaul
Copy link
Contributor

I don't know much about git, so I generally try to stay in my lane.
in general, I think it would be easier to take the pain and amend/fixup broken commits in pull requests and force-pushing so you get a "clean" PR/commit history, rather than adding commits to the PR and squashing everything when merging.

You make a good point, but the reality with open source is the contributors rarely do take the pain, and I don't have the time to do it for them so my habit is to squash-merge rather than tangle master more than it already is, especially while I'm also working on develop-1.9.

But now I know that's how you prefer to work, I'll try to make sure I don't squash your PRs in future!

Thanks so much for all these PRs - your contributions are greatly appreciated.

@SpacemanPaul SpacemanPaul merged commit 91e0f89 into opendatacube:develop-1.9 Aug 8, 2024
10 checks passed
@pjonsson pjonsson deleted the ci-tweaks-d19 branch August 8, 2024 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants