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

288 filip support right now only paylaod in mqtt custom / http custom but orion already supports json and ngsi as further options #296

Conversation

SystemsPurge
Copy link
Collaborator

The following warning pops up "UserWarning: Field name "json" shadows an attribute in parent "Mqtt"" ( due to the json fields in MqttCustom and HttpCustom ). Since the Http/Mqtt models do not have an explicit json field, i take it this is coming from some field/property in the base model.
Tried to circumvent the warning by the use of aliases which obviously did not work.

Not sure what model to specify for the json payload type, i just put a Dict[str, any] for now ( despite number/string being valid json , the orion manual specifies that it needs to be a JSON object and not an object of primitive type )

Any input on these two points is welcome.

@SystemsPurge SystemsPurge requested a review from djs0109 July 1, 2024 09:20
SystemsPurge and others added 3 commits July 2, 2024 14:02
…w-only-paylaod-in-mqttCustom-/-httpCustom-but-Orion-already-supports-json-and-ngsi-as-further-options

# Conflicts:
#	filip/models/ngsi_v2/subscriptions.py
#	tests/models/test_ngsi_v2_subscriptions.py
@djs0109
Copy link
Contributor

djs0109 commented Jul 9, 2024

closes #288

@djs0109
Copy link
Contributor

djs0109 commented Jul 9, 2024

I think it would be great if we can test the functionality of these fields too. @RCX112 Could you please implement a test that uses json, ngsi, payload for mqttCustom? An example can be found here

def test_notification(self):

@RCX112
Copy link
Collaborator

RCX112 commented Jul 12, 2024

The following warning pops up "UserWarning: Field name "json" shadows an attribute in parent "Mqtt"" ( due to the json fields in MqttCustom and HttpCustom ). Since the Http/Mqtt models do not have an explicit json field, i take it this is coming from some field/property in the base model. Tried to circumvent the warning by the use of aliases which obviously did not work.

Not sure what model to specify for the json payload type, i just put a Dict[str, any] for now ( despite number/string being valid json , the orion manual specifies that it needs to be a JSON object and not an object of primitive type )

Any input on these two points is welcome.

@SystemsPurge When I implemented the HttpCustom model I also had the same warnings. I simply supressed them with the warnings library, because the json() method of pydantic is already outdated anyway:

import warnings

# The pydantic models still have a .json() function, but this method is deprecated.
warnings.filterwarnings("ignore", category=UserWarning,
                        message='Field name "json" shadows an attribute in parent "Http"')
warnings.filterwarnings("ignore", category=UserWarning,
                        message='Field name "json" shadows an attribute in parent "Mqtt"')

@RCX112 RCX112 requested a review from djs0109 July 15, 2024 09:25
@RCX112
Copy link
Collaborator

RCX112 commented Jul 15, 2024

@djs0109 Which version of Orion do we currently use for the tests, because the custom notifications don't work for version 3.7 for example?

@djs0109
Copy link
Contributor

djs0109 commented Jul 17, 2024

Hi @RCX112 , it should be Orion >= 3.8, I have tested with 3.8.1

Copy link
Contributor

@djs0109 djs0109 left a comment

Choose a reason for hiding this comment

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

@SystemsPurge @RCX112 LGTM, thank you!

@djs0109 djs0109 merged commit 399849a into master Jul 17, 2024
1 check passed
@djs0109 djs0109 deleted the 288-filip-support-right-now-only-paylaod-in-mqttCustom-/-httpCustom-but-Orion-already-supports-json-and-ngsi-as-further-options branch July 17, 2024 13:06
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