Skip to content

Update dependency versions#237

Merged
Ewen Cheslack-Postava (ewencp) merged 1 commit into
masterfrom
update-dependencies
Oct 2, 2020
Merged

Update dependency versions#237
Ewen Cheslack-Postava (ewencp) merged 1 commit into
masterfrom
update-dependencies

Conversation

@ewencp

Copy link
Copy Markdown
Contributor

I also added a couple of comments where we're constrained by py27 support.

@ewencp

Copy link
Copy Markdown
Contributor Author

Also, wrt testing, system tests passed with these updates.

@andrewegel

Andrew Egelhofer (andrewegel) commented Jul 30, 2020

Copy link
Copy Markdown
Contributor

Also, wrt testing, system tests passed with these updates.

The bump to paramiko 2.6, that was then released to kafka here: apache/kafka#8879 has since seen SSHExceptions being raised randomly in Kafka System Tests. While maybe a bump to 2.7 might fix that, I think we need some large amounts of data comparing pre-2.6 to 2.7 test results in order to feel safe about this change.

@ewencp

Copy link
Copy Markdown
Contributor Author

Also, wrt testing, system tests passed with these updates.

The bump to paramiko 2.6, that was then released to kafka here: apache/kafka#8879 has since seen SSHExceptions being raised randomly in Kafka System Tests. While maybe a bump to 2.7 might fix that, I think we need some large amounts of data comparing pre-2.6 to 2.7 test results in order to feel safe about this change.

Yeah, system tests here are pretty minimal and not anywhere close to, e.g., length of test in kafkatest (which might affect these types of exceptions). Happy to downgrade, though we will have to upgrade eventually.

I don't have much context on that specific issue, but I can do branch builds of kafka system tests with this branch of ducktape pulled in to verify. Probably a good idea anyway given all the other dependency upgrades.

All that said, do you have links to the specific issues? SSHException is very generic, so hard to understand what the issue might be. It could be legitimate and tests (or the fmwk) just need to handle them more gracefully, or they might be real issues with the commands being executed.

@stan-is-hate

Stanislav Vodetskyi (stan-is-hate) commented Jul 30, 2020

Copy link
Copy Markdown
Member

I can do branch builds of kafka system tests with this branch of ducktape pulled in to verify

I'm also doing this in the background (specifically between paramiko 2.3 and 2.6), but right now a bit swamped with other work, so the progress is slow there.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm fine with shipping this change into master while we figure out what we want to do with AK and 0.7.x

@ewencp

Copy link
Copy Markdown
Contributor Author

Andrew Egelhofer (@andrewegel) Do we have data (or a sense) for how frequent the SSHExceptions are? I'm happy to kick off some kafkatest runs with this same change and check results for SSHException, but would need a baseline number to compare to.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM. Let's get this merged.

@stan-is-hate

Copy link
Copy Markdown
Member

+1 to Andrew's comment - let's get it in master, but keep 0.7.x as it is.
For context, our team is working on updating both AK as well as internal testing tools to py3 this quarter, so we'll be able to stop supporting py2 at some point over the next couple of months.

@edenhill

Copy link
Copy Markdown

stan-confluent We want to push out a new release of ducktape to unblock the py3 changes to AK system tests, and I think this PR should be part of the new release.
Should we bump ducktape to 0.8.0 so you can stay on <0.8?

@stan-is-hate

Copy link
Copy Markdown
Member

cc Ian McDonald (@imcdo) who's working on updating muckrake to py3
cc Nikesh Shettipalli (@nikesh0822) for visibility

Magnus Edenhill (@edenhill) what's blocking you from upgrading AK system tests to py3 first?
Ducktape is already on version 0.8.0 but we haven't cut a release branch yet. I don't see why not cut a 0.8.x branch if you need it - we haven't solved SSHExceptions issue, but we cannot forever stay on paramiko 2.3.2.
One feature in 0.8.0 that I'd ideally like to complete before doing a proper release is a better test suite support (we've added support for running test suites but didn't update the reporting) - but we can do it in 0.8.1 or 0.9.0.
Ewen Cheslack-Postava (@ewencp) what's your thoughts on this?

@edenhill

Copy link
Copy Markdown

stan-confluent AK system tests should use a released version of ducktape, not off master, for consistency and reproducability.

So it sounds like we can merge this PR, cut the 0.8.x release branch and do a 0.8.0 release, I'll get on it.

@stan-is-hate

Copy link
Copy Markdown
Member

Magnus Edenhill (@edenhill) yeah, if Ewen Cheslack-Postava (@ewencp) is also fine with it, sure. I was just curious what was stopping you from using py3 with ducktape 0.7.x?

@ivandasch

Copy link
Copy Markdown
Contributor

Hi! I would like to remind that there is no need of pysistence anymore. It should be removed from requirements.

@ivandasch

Ivan Daschinskiy (ivandasch) commented Oct 1, 2020

Copy link
Copy Markdown
Contributor

stan-confluent 0.7.x is not fully compatible with python3. I contributed few patches to fix this

@ewencp

Copy link
Copy Markdown
Contributor Author

Ivan Daschinskiy (@ivandasch) just because i filed this before that change was made, adding an update now

@ewencp
Ewen Cheslack-Postava (ewencp) deleted the update-dependencies branch October 2, 2020 00:53
@ewencp
Ewen Cheslack-Postava (ewencp) restored the update-dependencies branch March 5, 2021 07:49
@ewencp
Ewen Cheslack-Postava (ewencp) deleted the update-dependencies branch March 5, 2021 07:49
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.

5 participants