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

typeInformation contain global config values #1580

Merged
merged 5 commits into from
Feb 22, 2024

Conversation

KeshavSoni2511
Copy link
Contributor

Fix for issue #1515

@AlvaroVega
Copy link
Member

Could you please include the simplification of blocks referenced in #1515 (comment) ?

@KeshavSoni2511
Copy link
Contributor Author

Could you please include the simplification of blocks referenced in #1515 (comment) ?

Hi @AlvaroVega, I have done & tested the simplication in

const mustInsertTimeInstant =
typeInformation.timestamp !== undefined
? typeInformation.timestamp
: config.getConfig().timestamp !== undefined
? config.getConfig().timestamp
: false;

to

Screenshot 2024-02-16 174748

but test case httpBindings-test.js was failing.

The issue can be fixed by adding attribute "timestamp": "true" in provisionDeviceProduction.json. In case of iotagent-json it works without any test case failure.

Please confirm if this change is required or you have any other suggestion.

@KeshavSoni2511
Copy link
Contributor Author

Could you please include the simplification of blocks referenced in #1515 (comment) ?

Hi @AlvaroVega, I have done & tested the simplication in

const mustInsertTimeInstant =
typeInformation.timestamp !== undefined
? typeInformation.timestamp
: config.getConfig().timestamp !== undefined
? config.getConfig().timestamp
: false;

to

Screenshot 2024-02-16 174748

but test case httpBindings-test.js was failing.

The issue can be fixed by adding attribute "timestamp": "true" in provisionDeviceProduction.json. In case of iotagent-json it works without any test case failure.

Please confirm if this change is required or you have any other suggestion.

Hi @AlvaroVega, please review my understanding. Thanks

@fgalan
Copy link
Member

fgalan commented Feb 21, 2024

Hi @AlvaroVega, please review my understanding. Thanks

It seems there is a conflict on CHANGES_NEXT_RELEASE. This has to be solved (it's easy :) before merging.

@AlvaroVega
Copy link
Member

Could you please include the simplification of blocks referenced in #1515 (comment) ?

Hi @AlvaroVega, I have done & tested the simplication in

const mustInsertTimeInstant =
typeInformation.timestamp !== undefined
? typeInformation.timestamp
: config.getConfig().timestamp !== undefined
? config.getConfig().timestamp
: false;

to
Screenshot 2024-02-16 174748
but test case httpBindings-test.js was failing.
The issue can be fixed by adding attribute "timestamp": "true" in provisionDeviceProduction.json. In case of iotagent-json it works without any test case failure.
Please confirm if this change is required or you have any other suggestion.

Hi @AlvaroVega, please review my understanding. Thanks

Good news about iotagent-json tests works without any modifications after simplifying checks in iotagent-node-lib. I've see not problem in perform another PR to modify iotagent-ul tests in the way you describe (change timestamp to false in https://github.com/telefonicaid/iotagent-ul/blob/master/test/deviceProvisioning/provisionDeviceProduction.json and complete current PR with the simplifications in related checks

@KeshavSoni2511
Copy link
Contributor Author

Good news about iotagent-json tests works without any modifications after simplifying checks in iotagent-node-lib. I've see not problem in perform another PR to modify iotagent-ul tests in the way you describe (change timestamp to false in https://github.com/telefonicaid/iotagent-ul/blob/master/test/deviceProvisioning/provisionDeviceProduction.json and complete current PR with the simplifications in related checks

Hi @AlvaroVega, I have performed iotagent-ul test with "timestamp": "true" in https://github.com/telefonicaid/iotagent-ul/blob/master/test/deviceProvisioning/provisionDeviceProduction.json as global config value contain "timestamp": "true" and test case passed with the proposed change. Please confirm my understanding if I can proceed further and raise another PR on iotagent-ul. Please confirm if you have any other suggestion. Thanks

@AlvaroVega
Copy link
Member

Good news about iotagent-json tests works without any modifications after simplifying checks in iotagent-node-lib. I've see not problem in perform another PR to modify iotagent-ul tests in the way you describe (change timestamp to false in https://github.com/telefonicaid/iotagent-ul/blob/master/test/deviceProvisioning/provisionDeviceProduction.json and complete current PR with the simplifications in related checks

Hi @AlvaroVega, I have performed iotagent-ul test with "timestamp": "true" in https://github.com/telefonicaid/iotagent-ul/blob/master/test/deviceProvisioning/provisionDeviceProduction.json as global config value contain "timestamp": "true" and test case passed with the proposed change. Please confirm my understanding if I can proceed further and raise another PR on iotagent-ul. Please confirm if you have any other suggestion. Thanks

Yes, you can proced it with another PR for iotagent-ul

CHANGES_NEXT_RELEASE Outdated Show resolved Hide resolved
Changes done as per feedback received

Co-authored-by: Fermín Galán Márquez <fgalan@users.noreply.github.com>
Copy link
Member

@fgalan fgalan left a comment

Choose a reason for hiding this comment

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

LGTM

@fgalan
Copy link
Member

fgalan commented Feb 22, 2024

Thanks for the contribution!

@fgalan fgalan merged commit 18524dc into telefonicaid:master Feb 22, 2024
8 checks passed
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.

None yet

3 participants