-
Notifications
You must be signed in to change notification settings - Fork 544
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
Remove different build matrix selection for develop branches #1138
Conversation
…ke to try and use the smoke tests for as much as we reasonably can.
Right now the Jenkinsfile for the Python driver uses a different build matrix for feature branches named to match a PYTHON JIRA ticket. The build matrix in question doesn't really make sense for our plans going forward (it still includes Python 2.7 builds) and we do have the SMOKE build matrix which more naturally corresponds to the versions we'd like to support (and is already used as the default build matrix for most Jenkins builds). Goal here is basically to remove the special-case handling for branches matching this naming pattern. Going forward we may want to remove the DEVELOP matrix all together, but honestly I'd expect something like that to be handled in a larger-scale refactoring of the various matrices defined in the Jenkinsfile. For now, I just want to remove this special behaviour... I really want Jenkins feature branch builds to use the same matrix that will be used when we merge that feature branch into master. |
Worth mentioning here: the build for "develop branches" actually fails now due to an issue with something on 2.7:
Looks to be a transitive dependency issue with something not supporting 2.7.x anymore (typing is a Python3 module). I've dug into this enough to identify the above but that's about it; given that 2.7.x support will be going away there's no obvious reason to investigate this any further. And since that's the case there's no obvious reason to continue to execute anything on Jenkins against 2.7.x. |
Jenkinsfile
Outdated
if (env.BRANCH_NAME.contains("long")) { | ||
profile = "FULL" | ||
} | ||
if (env.BRANCH_NAME.contains("long")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove this if
as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's an interesting question.
There's going to be some changes to all these build matrices to support our new version support policy for the Python driver; from the new release on a given driver release will only support Python versions that aren't out-of-support at the time of release. So 2.7, 3.5 and 3.6 will be removed from all five build matrices, for instance. There's also a legit question for me about whether we really need five distinct build matrices... that seems like a lot.
But as I thought about it you're actually asking a different question; if we're already moving away from branch name correlation to a build matrix why not sever that tie completely? I think this is prolly right; I don't really like the idea that a branch name magically correlates to a specific build matrix. Seems better to maybe run other matrices on regular schedules and support ad-hoc runs if somebody really wants to run something now.
Whaddya think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we're already moving away from branch name correlation to a build matrix why not sever that tie completely
Exactly, if a branch gets named "something-long" and the build starts failing we may take a lot of time to realize it's using a completely different build because of this if
, I'd just remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sold. I was originally avoiding making this PR too big under the assumption that the overall build matrix would have to be revisited once we change supported versions of the Python runtime but as discussed above that concern isn't explicitly connected to what you're advocating here @joao-r-reis .
I'll make the change.
"SERVER": ['3.11', '4.0', 'dse-6.8.30'], | ||
"RUNTIME": ['3.7.7', '3.8.3'], | ||
"SERVER": DEFAULT_CASSANDRA.takeRight(2) + DEFAULT_DSE.takeRight(1), | ||
"RUNTIME": DEFAULT_RUNTIME.takeRight(2), | ||
"CYTHON": ["False"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just reducing some redundancy while we're here + making things a bit more declarative
@@ -33,30 +33,34 @@ slack = new Slack() | |||
// | |||
// Smoke tests are CI-friendly test configuration. Currently-supported Python version + modern C*/DSE instances. | |||
// We also avoid cython since it's tested as part of the nightlies. | |||
DEFAULT_CASSANDRA = ['2.1', '2.2', '3.0', '3.11', '4.0'] | |||
DEFAULT_DSE = ['dse-5.0.15', 'dse-5.1.35', 'dse-6.0.18', 'dse-6.7.17', 'dse-6.8.30'] | |||
DEFAULT_RUNTIME = ['2.7.18', '3.5.9', '3.6.10', '3.7.7', '3.8.3'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a little strange to keep 2.7.x + 3.5.x + 3.6.x around here. My rationale is that this PR is really about removing the special handling of "develop" branches + some normalization for how versions are specified. Actual changes in versions we test against will be done in a PR explicitly focused on that role (and should be accompanied by corresponding changes to the runners).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great way to keep these versions in a single place, TIL!
@joao-r-reis Quick ping; are you good with this change as it stands? |
…sync_with_upstream_3.29.1 version 3.28.0 * tag '3.28.0' of https://github.com/datastax/python-driver: Release 3.28.0: changelog & version PYTHON-1352 Add vector type, codec + support for parsing CQL type (datastax#1161) Update docs.yaml to point to most recent 3.27.0 docs changes CONN-38 Notes for 3.27.0 on PYTHON-1350 (datastax#1166) PYTHON-1356 Create session-specific protocol handlers to contain session-specific CLE policies (datastax#1165) PYTHON-1350 Store IV along with encrypted text when using column-level encryption (datastax#1160) PYTHON-1351 Convert cryptography to an optional dependency (datastax#1164) Jenkinsfile cleanup (datastax#1163) PYTHON-1343 Use Cython for smoke builds (datastax#1162) Don't fail when inserting UDTs with prepared queries with some missing fields (datastax#1151) Revert "remove unnecessary import __future__ (datastax#1156)" docs: convert print statement to function in docs (datastax#1157) remove unnecessary import __future__ (datastax#1156) Update docs.yaml to include recent fixes to CLE docs Fix for rendering of code blocks in CLE documentation (datastax#1159) DOC-3278 Update comment for retry policy (datastax#1158) DOC-2813 (datastax#1145) Remove different build matrix selection for develop branches (datastax#1138)
Goal here is to use the smoke tests for as many build cases as we reasonably can