-
Notifications
You must be signed in to change notification settings - Fork 192
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
#2144 Enhance Static Properties by UI validation V2 #2443
Conversation
@IsaakKrut, there seems to be an issue with the unit test. Do you have an idea what is going wrong. |
@tenthe The 3 values(serverAddressValue, hostValue, portValue) from the extractor you are using in OpcUaAdapterMigrationV2 have to be present in the extractor. Without them I'm getting NullPointerException in the AbstractParameterExtractor.singleValueParameter method call. Could we instantiate an extractor with those properties inside the test? And if so, are they gonna be available during actual migration as well? |
@IsaakKrut, I tested the migration and there I did not get any exceptions. |
@tenthe All these 3 fields (serverAddressValue, hostValue, portValue) are going to have a value during migration, right? If so we can add the defaultValue parameter for host and port for version 2 config in OpcUaAdapterVersionedConfig on lines 138, 140. And create an extractor inside the test with the same default values. Ran the test with those changes and it passed |
Ok, great. Can you add your changes. Then we can close this PR. |
Created a separate PR to update this branch since I don't have push access |
Hello there 👋 |
Hi @IsaakKrut, |
Hi @IsaakKrut, I have looked through the changes. So far I have assumed that we have to create a new static property. I have adapted code acordingly. Cheers, |
Hi @tenthe, Thanks for the update, looks much cleaner. Both the implementation and the tests. I tested locally and it works as expected |
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.
Looks good to me. Thanks a lot @IsaakKrut and sorry that this took so long!
Purpose
Remarks
PR introduces (a) breaking change(s): <yes/no>
PR introduces (a) deprecation(s): <yes/no>