Add support of field relashionsips on egg #5
Conversation
I also set a default values for the optional fields since a field must have a default value when using dataclass.
| @@ -229,7 +240,8 @@ class Egg: | |||
| startup: str | |||
| script: EggScript | |||
| created_at: str | |||
| updated_at: Optional[str] | |||
| updated_at: Optional[str] = None | |||
| relationships: Optional[dict] = None | |||
There was a problem hiding this comment.
Shouldn't this be Optional[EggRelationships]?
There was a problem hiding this comment.
Oh my bad you right! Ok so the problem get more complicated that i was thinking. How you implemented for now doesn't give the ability to automatically use the type EggRelationships and will return a dict instead which make the typing wrong.
Maybe your usage of the library would be to convert it later? For now I removed it because I'm not sure if this is usefull or no but I think kept the default value for the Optional type is a good thing.
On one of my library i use https://github.com/konradhalas/dacite which could be a perfect usage here, but after some try a lot of the typing in your types classes are incompatible with the current version of pterodactyl API sadly. The only "problem" I can see is: this will be a breaking change since it will automatically return the correct type. For example Egg.config will return an EggConfiguration object instead of a dict, while it's what the typing say it will do.
I can make a PR for that if you want to test how it look on your end.
There was a problem hiding this comment.
If it's returning a type that isn't what the type definition says, it's a bug, so I think it would be fine to implement this without it being a breaking change.
There was a problem hiding this comment.
Hum ok, so what I propose then is: we can merge this one. I removed the EggRelationships class for now and will do another PR with everything patched then.
There was a problem hiding this comment.
There's no need to remove it entirely – you can just implement the patch for type conversions in another PR. This can be closed for now.
|
Closing as per #5 (comment). |
This add support for the field
relashionsipson theEggmodels, useful for example when including more fields when using theincludeparameter from the eggs management endpointsI also set a default values for the optional fields since a field must have a default value when using dataclass.
I also added a default argument for the field with the type
Optional, sincedataclassneed a default value i think this will be requirement for the PyPI release #4