django-polymorphic icon indicating copy to clipboard operation
django-polymorphic copied to clipboard

Adding ppc64le architecture support on travis-ci

Open kishorkunal-raj opened this issue 3 years ago • 9 comments

Hi, I had added ppc64le(Linux on Power) architecture support on travis-ci in the branch and looks like its been successfully added. I believe it is ready for the final review and merge. The travis ci build logs can be verified from the link below. https://travis-ci.com/github/kishorkunal-raj/django-polymorphic/builds/189011390

Reason behind running tests on ppc64le: This package is included in the ppc64le versions of RHEL and Ubuntu - this allows the top of tree to be tested continuously as it is for Intel, making it easier to catch any possible regressions on ppc64le before the distros begin their clones and builds. This reduces the work in integrating this package into future versions of RHEL/Ubuntu.

Please have a look.

Regards, Kishor Kunal Raj

kishorkunal-raj avatar Oct 09 '20 11:10 kishorkunal-raj

Codecov Report

Merging #469 into master will decrease coverage by 0.19%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #469      +/-   ##
==========================================
- Coverage   88.83%   88.64%   -0.20%     
==========================================
  Files          34       34              
  Lines        2616     2616              
==========================================
- Hits         2324     2319       -5     
- Misses        292      297       +5     
Impacted Files Coverage Δ
polymorphic/query.py 89.31% <0.00%> (-2.14%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b4efb59...aa576e6. Read the comment docs.

codecov[bot] avatar Oct 09 '20 11:10 codecov[bot]

Any views on the requested PR?

kishorkunal-raj avatar Oct 19 '20 06:10 kishorkunal-raj

@chrisglass What did you think about this? From my POV I don't know if this project wants to commit to supporting power on linux. I think it would be easier to have architecture agnostic testing (like we currently have). I might not be understanding the benefits of adding this architecture being specifically tests.

@kishorkunal-raj What are the benefits from adding this as a supported architecture and does django upstream test for this? I wouldn't want us to be in a position where we're supporting something that isn't supported upstream

AdamDonna avatar Oct 19 '20 07:10 AdamDonna

@chrisglass I'd like to close/reject this change I don't think there's sufficient justification to add ppc64le support

AdamDonna avatar Oct 30 '20 09:10 AdamDonna

@chrisglass I'd like to close/reject this change I don't think there's sufficient justification to add ppc64le support

@gerrith3 Will you please help to understand reason behind adding ppc64le architecture

kishorkunal-raj avatar Nov 04 '20 07:11 kishorkunal-raj

@chrisglass @AdamDonna Adding automated CI simply ensures that the project (which is already shipped in several ppc64le distros) will continue to build and run without issue. The additional testing is minor, IBM covers the cost of the hardware, and allows us to catch if this package or any dependencies are missing or not working. It works today based on @kishorkunal-raj s testing, this would give quick feedback to anyone that added an arch dependent patch that broke on Power. And odds are that the regression would be unintentional and probably easily fixed at that point, rather than waiting until each distro gets around to testing. Django is not yet testing on ppc64le since they have their own Jenkins server without ppc64le hardware access. We'll work to get them access at some point in this process.

gerrith3 avatar Nov 05 '20 01:11 gerrith3

@gerrith3 Yeah I think you might have nailed my reservations when you said

Django is not yet testing on ppc64le since they have their own Jenkins server without ppc64le hardware access.

I think this is something we should do once that's achieved otherwise we might appear to have committed to supporting ppc64le when it's not achievable (since it's not supported upstream).

Do you have a timeline on when django will adopt ppc64le support so we can talk with the core maintainers of this repo, and add support?

AdamDonna avatar Nov 05 '20 12:11 AdamDonna

I finally had time to look at this.

I'm not against adding travis build configurations for ppc64le at all, however, I don't really understand why this is useful to do for this project in particular: it's a pure-python project, that should therefore be platform independant as long as the libraries it's using are ported fine.

Do you mean to say that you would like to build this project for ppc64le to use it as the "top of the tree" to shake a large amount of dependencies?

chrisglass avatar Nov 05 '20 14:11 chrisglass

@chrisglass as you said "as long as the libraries it's using are ported fine" - which is definitely part of the issue we are trying to address. In reality today, 85-90% of everything "just works" for ppc64le users/customers today, but whenever there is an issue, my team is called in to port. We are trying to do a more proactive, wide spread set of testing, starting at the base. As you are probably aware, debian and canonical/ubuntu use our data farms and supposedly test everything in the distro, fixing/patching/updating things as they need, and usually upstreaming any changes when they can. We are trying to broaden the base testing, e.g. items in the distro, and for now, specifically those that use travis ci on Intel, to ensure the full stack of dependencies all work and that they are validated at top of tree. With testing at top of tree, we get more automated validation and also immediate notification to any developers when they add/change anything to break architecture independence. So, yes, shaking things out below is definitely an intended goal and happy side effect of adding this additional testing.

gerrith3 avatar Nov 05 '20 21:11 gerrith3

Given the jazz band migration which appears to require a migration to github actions, I suspect we can close this PR.

AdamDonna avatar Nov 23 '23 10:11 AdamDonna

I'm going to close this PR. We can reopen it and discuss if it's necessary

AdamDonna avatar Nov 28 '23 00:11 AdamDonna