Add more events when creating a machine with hetzner cloud#59861
Add more events when creating a machine with hetzner cloud#59861dithmer wants to merge 24 commits intosaltstack:masterfrom
Conversation
krionbsd
added
the
needs-testcase
|
Hello! I feel like there is already the If there is still something wanted, please let me know your ideas of how I can test the events I trigger. Best regards |
…or-ssh-event-to-hetzner
…sso-mobile/salt into add-waiting-for-ssh-event-to-hetzner
…or-ssh-event-to-hetzner
…sso-mobile/salt into add-waiting-for-ssh-event-to-hetzner
|
I've just added tests for the timeout and added the check if server is running via a |
Hi! I'm your friendly PR bot!You might be wondering what I'm doing commenting here on your PR. Yes, as a matter of fact, I am... I'm just here to help us improve the documentation. I can't respond to Okay... so what do you do? I detect modules that are missing docstrings or "CLI Example" on existing docstrings! So what does that have to do with my PR? I noticed that in this PR there are some files changed that have some of these Okay, what are they? Well, my favorite, is that since you were making changes here I'm hoping that If I can, then what? Well, you can either add them to this PR or add them to another PR. Either way is fine! Well... what if I can't, or don't want to? That's also fine! We appreciate all contributions to the Salt Project. If you Whatever approach you decide to take, just drop a comment here letting us know! Detected Issues (click me)[INFO] Initializing environment for https://github.com/saltstack/pip-tools-compile-impersonate. [INFO] Initializing environment for https://github.com/asottile/pyupgrade. [INFO] Initializing environment for https://github.com/saltstack/pre-commit-remove-import-headers. [INFO] Initializing environment for https://github.com/s0undt3ch/salt-rewrite. [INFO] Initializing environment for https://github.com/timothycrosley/isort. [INFO] Initializing environment for https://github.com/timothycrosley/isort:toml. [INFO] Initializing environment for https://github.com/psf/black. [INFO] Initializing environment for https://github.com/asottile/blacken-docs. [INFO] Initializing environment for https://github.com/asottile/blacken-docs:black==21.7b0. [INFO] Initializing environment for https://github.com/PyCQA/bandit. [INFO] Initializing environment for https://github.com/saltstack/invoke-pre-commit. [INFO] Initializing environment for https://github.com/saltstack/invoke-pre-commit:blessings,distro,jinja2,msgpack,pyyaml. [INFO] Initializing environment for https://github.com/saltstack/mirrors-nox. [INFO] Initializing environment for https://github.com/saltstack/mirrors-nox:pip>=20.2.4,<21.2,setuptools<58.0. [INFO] Installing environment for https://github.com/saltstack/invoke-pre-commit. [INFO] Once installed this environment will be reused. [INFO] This may take a few minutes... Check Known Missing Docstrings...........................................Failed - hook id: invoke - duration: 1.51s - exit code: 1 Thanks again! |
|
@dithmer can you update the PR to the current version of the master? We are looking into these PR's and it looks like this one has some errors in lint but because of the last time the tests were run we can't see what those errors were. |
|
Taking a look at this right now - I have a question about the test that I want to confirm real quick. |
waynew
left a comment
waynew
left a comment
There was a problem hiding this comment.
Okay, question was answered. I don't think that my suggestions are necessarily blocking now that they've been documented here in the issue.
| with pytest.raises(TimeoutError): | ||
| with Timeout(seconds=3): | ||
| hetzner.create(vm) |
There was a problem hiding this comment.
suggestion (non-blocking) it would be helpful to mention something either in the comments or in the actual test name. I would say something like:
def test_when_server_status_is_not_running_create_should_never_return(vm):
Alternatively the comment could be
Create will never return unless the .status is "running", so we need a Timeout that will interrupt the loop in
.create.
I'm not currently familiar with the surrounding code, so I'm assuming that the cloud code will already timeout this call?
|
bump @dithmer are you able to come back to this one and implement the feedback and fix up the tests? |
Hi, I would love to take on it again. I just didn't feel that it was too urgent, cause this issue is already open for about 2 years. It will probably take my attention during the next week! Have a nice day! |
|
Will take a few more days. But I'm on it! |
…hmer/salt into add-waiting-for-ssh-event-to-hetzner
Add waiting for ssh event to hetzner
|
Hi @Ch3LL I am trying to launch the tests locally but whenever I follow the tutorial for installing (https://docs.saltproject.io/en/master/topics/development/hacking.html#installing-salt-for-development) it installs salt in a version (3000.1) which is not compatible with the tests resulting in an error message: What am I missing here? Branch was rebased with master and I've already tried to clean cache/remove the whole virtualenv. Best regards |
|
Can you ensure you have rebased the lastest changes off of master? I've never seen that before. |
Absolutely. Couldnt find out whats the problem yet. |
|
One other thing you can try is to remove your also @s0undt3ch any ideas at first glance on that error posted above? |
|
Make sure you have all the tags and yes, removing |
|
Closing this due to inactivity. Anyone should feel free to re-open it if they want to see it through to the end in one release cycle. |
9 participants
What does this PR do?
It adds events and a waiting routing for the creation of cloud machines via hetzner cloud driver to react to them via the reactor system of saltstack.
Commits signed with GPG?
No