-
Notifications
You must be signed in to change notification settings - Fork 83
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
Added json method to Entity #326
Conversation
@Ichimonji10 could I ask you for your opinion here? |
1f53a0c
to
c4d3dd3
Compare
I think, based on the output below, I will also need
|
@ehelms I suppose you want nested strutures, right? Using default _parse function it just tranform complex structures on their respective id. Check Repository parsed to id. If nested is what you want I can try it, I mean, if you would expect something like: { # package regular fields
{'repository': {'id' : 1}}
} |
@ehelms I played with the nested structure on last commit, check if it works for you |
6689d84
to
73b0207
Compare
ba5de65
to
1b4a70f
Compare
@ehelms last time we spoke it was working in your tests. Is it still working? Let me know if it is so folk can merge and made a new version available on pypi ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK pending some small comments
'name': u'sclo-git25' | ||
} | ||
cfg = config.ServerConfig( | ||
url='https://sat-r220-02.lab.eng.rdu2.redhat.com/', verify=False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove internal url, we shouldn't use any of such in our public repos
'name': u'sclo-git25' | ||
} | ||
cfg = config.ServerConfig( | ||
url='https://sat-r220-02.lab.eng.rdu2.redhat.com/', verify=False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove internal url
@@ -510,6 +511,44 @@ def __repr__(self): | |||
) | |||
) | |||
|
|||
def to_json(self): | |||
r"""Create a JSON encoded string with Entity properties. Ex: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty line needed here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw what does that r"""
format stands for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without the raw string you could use \\**kwargs
, that should work.
>>> org.to_json() | ||
'{"id": 1, "name": "Nailgun Org"}') | ||
|
||
:return: str |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And another empty line after this one
and build a list calling the same method on each object on OneToMany | ||
fields. | ||
|
||
:return: dct |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another empty line needed here:
System Message: WARNING/2 (<string>, line 8)
Field list ends without a blank line; unexpected unindent.
""" Transforms obj into a json serializable object. | ||
|
||
:param obj: entity or any json serializable object | ||
:return: serializable object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty line is needed after this line:
System Message: WARNING/2 (<string>, line 5)
Field list ends without a blank line; unexpected unindent.
... } | ||
>>> org = entities.Organization(config.ServerConfig('foo'), \*\*kwargs) | ||
>>> org.to_json() | ||
'{"id": 1, "name": "Nailgun Org"}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: closing bracket is probably redundant here
if isinstance(field, OneToOneField): | ||
json_dct[field_name] = values[field_name].to_json_dict() | ||
elif isinstance(field, OneToManyField): | ||
json_dct[field_name] = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is not covered by tests
Btw please include transformation result for one 'real' entity with numerous fields/subentities. |
7afdcc1
to
a001a90
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK pending the last nitpick
>>> org.to_json() | ||
'{"id": 1, "name": "Nailgun Org"}' | ||
|
||
:return: str |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the last one empty line missing after this one
@renzon you misunderstood my suggestion, i was asking about adding a blank line after
However, I've checked sphinx and it didn't throw any warning, build succeeded and resulting doc had no issues, so i can't see the reason to block this PR anymore, my suggestion is definitely optional. |
@renzon can you put some demonstration on Nailgun docs? some examples on how to use and also output examples of json methods. maybe in http://nailgun.readthedocs.io/en/latest/examples.html (https://github.com/SatelliteQE/nailgun/blob/master/docs/examples.rst) |
ACK |
Shortlog of commits since last release: Andrii Balakhtar (4): Fix read() of hostgroup without content_source, lce or cv for 6.1 Increase timeout for manifest delete/refresh (SatelliteQE#359) Provide default name length & type for ComputeResource entities (SatelliteQE#369) Implemented Interface entity with all its fields and basic operations (SatelliteQE#373) Djebran Lezzoum (1): Merge pull request SatelliteQE#380 from svtkachenko/ct_update_payload Elyézer Rezende (1): Update travis config Oleksandr Shtaier (8): Merge pull request SatelliteQE#341 from sghai/update-test Merge pull request SatelliteQE#342 from renzon/332 Merge pull request SatelliteQE#344 from elyezer/update-travis Merge pull request SatelliteQE#352 from svtkachenko/sv_payload Merge pull request SatelliteQE#345 from abalakh/fix_hg_61_workaround Merge pull request SatelliteQE#366 from svtkachenko/update_template_kind Merge pull request SatelliteQE#361 from svtkachenko/update_sv Merge pull request SatelliteQE#383 from svtkachenko/63_image Renzo Nuccitelli (3): Added json method to Entity (SatelliteQE#326) Merge pull request SatelliteQE#347 from svtkachenko/add_image Implemented Entity __eq__ method comparing all fields (SatelliteQE#350) Stanislav Tkachenko (10): Add content related helpers for Repository entity. (SatelliteQE#327) Add Create and Delete mixins to Smart Proxy (SatelliteQE#343) Add mixins to Image Override create_payload and upload_payload for SmartVariable. Update Host.puppet_class field name. (SatelliteQE#346) Removed overriden read method for Smart Variable Update TemplateKind entity Add ProvisioningTemplate. (SatelliteQE#365) Fix update_payload method for ConfigTemplate/ProvisioningTemplate [6.2.z] Update Image and Host entities (SatelliteQE#356) oshtaier (2): Add UpdateMixin for CV Filter Rules entity Add host count field to CV entity renzon (5): Fixed ConfigTemplate create and update with TemplateCombination ListFiled handled on _payload api fixed and methods GET and DELETED added for TemplateCombination api fixed and methods GET and DELETED added for TemplateCombination api fixed and methods GET and DELETED added for TemplateCombination sghai (5): Updated discovery_rule entity with org & location (SatelliteQE#320) Added a new field root_pass to hostgroup entity (SatelliteQE#336) Added unit test for root_pass from hostgroup entity Merge pull request SatelliteQE#338 from oshtaier/cvf_update_mixin Merge pull request SatelliteQE#340 from oshtaier/cv_field
* Added nested json parsing to Entity close SatelliteQE#323 * Added to_json_serializable function close SatelliteQE#323 * Fixed last nitpick on docstrings * Added docs for to_json_serializable function
Shortlog of commits since last release: Andrii Balakhtar (4): Fix read() of hostgroup without content_source, lce or cv for 6.1 Increase timeout for manifest delete/refresh (SatelliteQE#359) Provide default name length & type for ComputeResource entities (SatelliteQE#369) Implemented Interface entity with all its fields and basic operations (SatelliteQE#373) Djebran Lezzoum (1): Merge pull request SatelliteQE#380 from svtkachenko/ct_update_payload Elyézer Rezende (1): Update travis config Oleksandr Shtaier (8): Merge pull request SatelliteQE#341 from sghai/update-test Merge pull request SatelliteQE#342 from renzon/332 Merge pull request SatelliteQE#344 from elyezer/update-travis Merge pull request SatelliteQE#352 from svtkachenko/sv_payload Merge pull request SatelliteQE#345 from abalakh/fix_hg_61_workaround Merge pull request SatelliteQE#366 from svtkachenko/update_template_kind Merge pull request SatelliteQE#361 from svtkachenko/update_sv Merge pull request SatelliteQE#383 from svtkachenko/63_image Renzo Nuccitelli (3): Added json method to Entity (SatelliteQE#326) Merge pull request SatelliteQE#347 from svtkachenko/add_image Implemented Entity __eq__ method comparing all fields (SatelliteQE#350) Stanislav Tkachenko (10): Add content related helpers for Repository entity. (SatelliteQE#327) Add Create and Delete mixins to Smart Proxy (SatelliteQE#343) Add mixins to Image Override create_payload and upload_payload for SmartVariable. Update Host.puppet_class field name. (SatelliteQE#346) Removed overriden read method for Smart Variable Update TemplateKind entity Add ProvisioningTemplate. (SatelliteQE#365) Fix update_payload method for ConfigTemplate/ProvisioningTemplate [6.2.z] Update Image and Host entities (SatelliteQE#356) oshtaier (2): Add UpdateMixin for CV Filter Rules entity Add host count field to CV entity renzon (5): Fixed ConfigTemplate create and update with TemplateCombination ListFiled handled on _payload api fixed and methods GET and DELETED added for TemplateCombination api fixed and methods GET and DELETED added for TemplateCombination api fixed and methods GET and DELETED added for TemplateCombination sghai (5): Updated discovery_rule entity with org & location (SatelliteQE#320) Added a new field root_pass to hostgroup entity (SatelliteQE#336) Added unit test for root_pass from hostgroup entity Merge pull request SatelliteQE#338 from oshtaier/cvf_update_mixin Merge pull request SatelliteQE#340 from oshtaier/cv_field
close #323
Once the methods uses _payload, some Entities maybe need
override this to match names expected on API
I followed @elyezer tip and made a this POC that worked for simple properties: renzon@d092468.
The problem is when the default naming mapping is not correct with the expected for API. But in this case Subclasses can override _to_json_dict for the exceptions.
@ehelms and @rochacbruno could give it a try to check exceptions as they use it and creating issues here for them. What you guys think?