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

Add jobs to test builds on different distros #6499

Merged
merged 30 commits into from
Dec 1, 2022

Conversation

gurkanindibay
Copy link
Contributor

@gurkanindibay gurkanindibay commented Nov 17, 2022

With this PR, citus code will be tested in all packaging environments.
Sometimes, there can be compile errors which blocks packaging and in this case unplanned delays may occur.
By testing the code in packaging environments, I'm aiming to detect any compilation errors before packaging.

.github/workflows/packaging-test-pipelines.yml Outdated Show resolved Hide resolved
.github/workflows/packaging-test-pipelines.yml Outdated Show resolved Hide resolved
.github/workflows/packaging-test-pipelines.yml Outdated Show resolved Hide resolved
.github/workflows/packaging-test-pipelines.yml Outdated Show resolved Hide resolved
.github/workflows/packaging-test-pipelines.yml Outdated Show resolved Hide resolved
.github/workflows/packaging-test-pipelines.yml Outdated Show resolved Hide resolved
.github/workflows/packaging-test-pipelines.yml Outdated Show resolved Hide resolved
@onurctirtir onurctirtir changed the title Adds packaging test pipelines Add jobs to test builds on different distros Nov 17, 2022
@onurctirtir
Copy link
Member

onurctirtir commented Nov 17, 2022

Btw are those tests run in parallel to each other ? If not, that would affect development velocity very badly.

@gurkanindibay
Copy link
Contributor Author

gurkanindibay commented Nov 17, 2022

Btw are those tests run in parallel to each other ? If not, that would affect developer velocity very badly.

https://github.com/citusdata/citus/actions/runs/3489359842
it takes 4.5 minutes
it executes parallel

@gurkanindibay gurkanindibay force-pushed the gindibay/packaging-check-pipelines branch 2 times, most recently from bdfd625 to 4692339 Compare November 23, 2022 09:00
@gurkanindibay gurkanindibay force-pushed the gindibay/packaging-check-pipelines branch from 53f3c7a to bbad460 Compare November 23, 2022 09:33
Copy link
Member

@onurctirtir onurctirtir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. This PR adds 22 more jobs; 5 x 3 for rpm based distros, which is nice, and only 6 for deb based distros. Are we sure that we're testing deb based ones for all supported pg versions ?

  2. Also seeing lots of such warnings, can we fix them ? (as in https://github.com/citusdata/citus/actions/runs/3530829173)

wrn

  1. For those gcc warnings, are we sure that we're using the correct gcc version (as you discussed with @JelteF in Teams chat) ?

(Btw this really looks good overall and I think we're very close to merge :) )

.github/workflows/packaging-test-pipelines.yml Outdated Show resolved Hide resolved
src/bin/pg_send_cancellation/Makefile Outdated Show resolved Hide resolved
packaging_ignore.yml Outdated Show resolved Hide resolved
@gurkanindibay
Copy link
Contributor Author

gurkanindibay commented Nov 29, 2022

  1. This PR adds 22 more jobs; 5 x 3 for rpm based distros, which is nice, and only 6 for deb based distros. Are we sure that we're testing deb based ones for all supported pg versions ?
  2. Also seeing lots of such warnings, can we fix them ? (as in https://github.com/citusdata/citus/actions/runs/3530829173)

wrn

  1. For those gcc warnings, are we sure that we're using the correct gcc version (as you discussed with @JelteF in Teams chat) ?

(Btw this really looks good overall and I think we're very close to merge :) )

  1. Yes because the images are the actual packaging images. Whenever a new postgres version is released, I bake a new packaging image
  2. These warnings about a Github Action checkout task. Updated the version and now it is OK
  3. Yes. These are actual packages and the warnings are from the actual image that includes gcc that is the latest version that is being supported in the OS/release repo

@onurctirtir
Copy link
Member

  1. This PR adds 22 more jobs; 5 x 3 for rpm based distros, which is nice, and only 6 for deb based distros. Are we sure that we're testing deb based ones for all supported pg versions ?
  2. Also seeing lots of such warnings, can we fix them ? (as in https://github.com/citusdata/citus/actions/runs/3530829173)

wrn

  1. For those gcc warnings, are we sure that we're using the correct gcc version (as you discussed with @JelteF in Teams chat) ?

(Btw this really looks good overall and I think we're very close to merge :) )

  1. Yes because the images are the actual packaging images. Whenever a new postgres version is released, I bake a new packaging image
  2. These warnings about a Github Action checkout task. Updated the version and now it is OK
  3. Yes. These are actual packages and the warnings are from the actual image that includes gcc that is the latest version that is being supported in the OS/release repo
  1. I meant, I'm only seeing a single run for each one of those deb-all images, I believe we should have 3 jobs to test each one of them with 3 different posgtres versions ?

@gurkanindibay
Copy link
Contributor Author

  1. This PR adds 22 more jobs; 5 x 3 for rpm based distros, which is nice, and only 6 for deb based distros. Are we sure that we're testing deb based ones for all supported pg versions ?
  2. Also seeing lots of such warnings, can we fix them ? (as in https://github.com/citusdata/citus/actions/runs/3530829173)

wrn

  1. For those gcc warnings, are we sure that we're using the correct gcc version (as you discussed with @JelteF in Teams chat) ?

(Btw this really looks good overall and I think we're very close to merge :) )

  1. Yes because the images are the actual packaging images. Whenever a new postgres version is released, I bake a new packaging image
  2. These warnings about a Github Action checkout task. Updated the version and now it is OK
  3. Yes. These are actual packages and the warnings are from the actual image that includes gcc that is the latest version that is being supported in the OS/release repo
  1. I meant, I'm only seeing a single run for each one of those deb-all images, I believe we should have 3 jobs to test each one of them with 3 different posgtres versions ?

You're right. I changed to add all versions of Postgres for deb based distros now

@gurkanindibay gurkanindibay force-pushed the gindibay/packaging-check-pipelines branch from a0b65ed to d64abe7 Compare November 30, 2022 05:22
Copy link
Member

@onurctirtir onurctirtir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now, thank you for adding those jobs to Citus to early detect build time issues / before it's time to do a release 🚀.

Here is a checklist before merging, feel free to click check-mark symbols on each item as you move forward:

  • Now it's time to get in touch with people in engine chat to ask for help to get those build warnings fixed (to be able to merge this PR)
  • Please squash the commits before merging.
  • After merging the PR into main, please backport this at least to release-11.0 / release-11.1 branches (and optionally to release 10.2 too) so that we can early detect build-time problems / before doing a patch release for those versions.
    Those versions of Citus are still actively supported and hence it's quite likely that we might want to release new packages for them at some point.

.github/workflows/packaging-test-pipelines.yml Outdated Show resolved Hide resolved
src/bin/pg_send_cancellation/Makefile Outdated Show resolved Hide resolved
@gurkanindibay gurkanindibay force-pushed the gindibay/packaging-check-pipelines branch from 61877a8 to eb7fa64 Compare November 30, 2022 20:05
Copy link
Member

@hanefi hanefi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working functionality with several comments for increasing readability and maintainability.

.github/workflows/packaging-test-pipelines.yml Outdated Show resolved Hide resolved
.github/workflows/packaging-test-pipelines.yml Outdated Show resolved Hide resolved
.github/workflows/packaging-test-pipelines.yml Outdated Show resolved Hide resolved
.github/workflows/packaging-test-pipelines.yml Outdated Show resolved Hide resolved
.github/workflows/packaging-test-pipelines.yml Outdated Show resolved Hide resolved
.github/workflows/packaging-test-pipelines.yml Outdated Show resolved Hide resolved
.github/workflows/packaging-test-pipelines.yml Outdated Show resolved Hide resolved
gurkanindibay and others added 5 commits December 1, 2022 15:27
Co-authored-by: Hanefi Onaldi <Hanefi.Onaldi@microsoft.com>
Co-authored-by: Hanefi Onaldi <Hanefi.Onaldi@microsoft.com>
@gurkanindibay gurkanindibay merged commit c219360 into main Dec 1, 2022
@gurkanindibay gurkanindibay deleted the gindibay/packaging-check-pipelines branch December 1, 2022 16:11
gurkanindibay added a commit that referenced this pull request Dec 5, 2022
With this PR, citus code will be tested in all packaging environments. 
Sometimes, there can be compile errors which blocks packaging and in
this case unplanned delays may occur.
By testing the code in packaging environments, I'm aiming to detect any
compilation errors before packaging.

Co-authored-by: Onur Tirtir <onurcantirtir@gmail.com>
Co-authored-by: Hanefi Onaldi <Hanefi.Onaldi@microsoft.com>
gurkanindibay added a commit that referenced this pull request Dec 5, 2022
With this PR, citus code will be tested in all packaging environments. 
Sometimes, there can be compile errors which blocks packaging and in
this case unplanned delays may occur.
By testing the code in packaging environments, I'm aiming to detect any
compilation errors before packaging.

Co-authored-by: Onur Tirtir <onurcantirtir@gmail.com>
Co-authored-by: Hanefi Onaldi <Hanefi.Onaldi@microsoft.com>
gurkanindibay added a commit that referenced this pull request Dec 5, 2022
With this PR, citus code will be tested in all packaging environments. 
Sometimes, there can be compile errors which blocks packaging and in
this case unplanned delays may occur.
By testing the code in packaging environments, I'm aiming to detect any
compilation errors before packaging.

Co-authored-by: Onur Tirtir <onurcantirtir@gmail.com>
Co-authored-by: Hanefi Onaldi <Hanefi.Onaldi@microsoft.com>
gurkanindibay added a commit that referenced this pull request Dec 5, 2022
With this PR, citus code will be tested in all packaging environments. 
Sometimes, there can be compile errors which blocks packaging and in
this case unplanned delays may occur.
By testing the code in packaging environments, I'm aiming to detect any
compilation errors before packaging.

Co-authored-by: Onur Tirtir <onurcantirtir@gmail.com>
Co-authored-by: Hanefi Onaldi <Hanefi.Onaldi@microsoft.com>
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.

4 participants