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

Missing server create fields #94

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

cosmohdx
Copy link
Contributor

This commit adds the missing fields public_net and placement_group to server create functions.

@mikebarlow
Copy link

Any chance of getting this merged in and released soon?
Would save me a ton of time having to maintain a fork! 🙏

@LKaemmerling
Copy link
Collaborator

Sure sorry I missed it! Could you maybe add a test too? Just to make sure it works as expected :)

@mikebarlow
Copy link

mikebarlow commented Mar 3, 2023

The package as it stands doesn't even have tests for creating servers, so how do you know server creation works at the moment? 😉

@LKaemmerling
Copy link
Collaborator

@mikebarlow yes correct, however if there are new features it should be tried to introduce tests where they are missing :) And yes the create server part is not well testes.

@cosmohdx
Copy link
Contributor Author

The package as it stands doesn't even have tests for creating servers, so how do you know server creation works at the moment? 😉

I just used it. But you are right, tests are missing.
May I add them or will the tests just stay here for an other 8 month ;)

@LKaemmerling
Copy link
Collaborator

It would be good if there are tests :) even if they are only for your change.

@cosmohdx
Copy link
Contributor Author

I just checked, you have public_net already inside your tests. Only the parameter at creation was missing.

@cosmohdx
Copy link
Contributor Author

I added the https://docs.hetzner.cloud/#placement-groups section to the api for tests with the placement group. The placement group field at the server is only an id, needs to be existing, idk what kind of test you are expecting.

@LKaemmerling
Copy link
Collaborator

Hey @cosmohdx,

sorry i was on vacation. What i expect is simply that the whole passing through is tested, so that the expected values are on the correct places :) It looks like there a a few failing tests. Could you maybe fix them? Then everything is good to go :)

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.

3 participants