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

installation/await_install: Bump timeout for powepc #13885

Merged
merged 1 commit into from Dec 20, 2021

Conversation

foursixnine
Copy link
Member

@foursixnine foursixnine commented Dec 16, 2021

Installation is taking a bit longer nowdays, and while I'm not sure that it's due to network or some other issue, those extra 10 minutes should help, as most of the times... looks like await install is just shy of minutes before finishing the installation

https://openqa.suse.de/tests/7856741#step/await_install/69

VR: https://openqa.suse.de/t7867595

See also https://progress.opensuse.org/issues/104106

@foursixnine foursixnine added the Ready Ready for review label Dec 17, 2021
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

s/powepc/PowerPC/ or ppc or something. The problem is https://progress.opensuse.org/issues/102882 which is really specific to one rack in a server room used by the OSD infrastructure. I suggest to use TIMEOUT_SCALE in the worker specific config, not change the test code

Copy link
Contributor

@punkioudi punkioudi left a comment

Choose a reason for hiding this comment

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

lgtm!

@foursixnine
Copy link
Member Author

s/powepc/PowerPC/ or ppc or something. The problem is https://progress.opensuse.org/issues/102882 which is really specific to one rack in a server room used by the OSD infrastructure. I suggest to use TIMEOUT_SCALE in the worker specific config, not change the test code

I will still move forward, as the test normally takes 12m, and the extra timeout should cover for future problems in the network even after https://gitlab.suse.de/openqa/salt-pillars-openqa/-/merge_requests/375, or for the time it is reverted.

@foursixnine foursixnine merged commit e5a5d30 into os-autoinst:master Dec 20, 2021
@okurz
Copy link
Member

okurz commented Dec 20, 2021

You could have at least fixed the typo :,( I don't think it's a good style to merge despite someone requesting changes.

@foursixnine foursixnine deleted the tellmewhythistime branch December 20, 2021 12:31
@foursixnine
Copy link
Member Author

foursixnine commented Dec 20, 2021

You could have at least fixed the typo :(

Which typo? :) I hope you're seeing it now and not during your first review

@okurz
Copy link
Member

okurz commented Dec 20, 2021

The typo I mentioned in #13885 (review)

s/powepc/PowerPC/ or ppc or something.

Seems like you overlooked my first message. That's ok, mistakes happen. But even more so this is why I suggest to not merge despite changes requested.

@foursixnine
Copy link
Member Author

The typo I mentioned in #13885 (review)

s/powepc/PowerPC/ or ppc or something.

Seems like you overlooked my first message. That's ok, mistakes happen. But even more so this is why I suggest to not merge despite changes requested.

Ah, wasn't really clear what you meant, the second part of that comment diluted the possible meaning of it.

wrt merging despite changes requested, let's say that there's a calculated risk (if any) to simply waiting for a bit longer, instead of just waiting for the workaround.

git history will show how that there's a reason for that in the future, and that would be a better early warning in the future (including to check for typos before merging :).

@okurz
Copy link
Member

okurz commented Dec 20, 2021

sure, not the end of the world :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready Ready for review
Projects
None yet
4 participants