search

ni / measurement-plugin-python

Python framework to develop measurement plug-ins for NI application software. Contains sample measurement plug-ins for InstrumentStudio and TestStand.
MIT License
19 stars 15 forks source link

MeasurementService should provide more guidance about moving version number to the .serviceconfig file #908

Closed bkeryan closed 1 month ago

bkeryan commented 1 month ago

Bug Report

If the service specifies the version number in code but not in the serviceconfig file, the discovery service assumes that the service is unversioned and assigns it a default version number. If this doesn't match, calling the measurement service from TestStand will fail.

From @nick-beer

Yeah - in fact, passing the a version to the constructor is problematic if it is not also found in the .serviceconfig file. We should probably update the service.py code to enforce this. The problem is as follows:

  • You specify a version in your constructor, but not in your .serviceconfig file.
  • When the service is published to plugin library or statically installed, the DiscoveryService thinks the service is unversioned - because there's no version in the .serviceconfig file.
  • As a result, it registers it internally with a default version (used to be 1.0.0, but as of last night, it's now 0.0.1)
  • However, when the service starts itself and registers itself with discovery service, it uses this version that's passed in the constructor, and those versions don't match. This means DiscoveryService fails to see this as a valid registration/resolution, and the client is left hanging, waiting for that valid registration.

... I think an appropriate fix is to allow the following and raise an error for anything else:

  • No version specified in either the constructor or serviceconfig. This will just get the default version
  • No version specified in the constructor, but a version specified in the serviceconfig.
  • A version specified in the constructor, and that same version is specified in the serviceconfig

If someone has an existing service with a version in the constructor (effectively all services), they'll be required to make some kind of change when updating SDK versions. This is unfortunate, but because DiscoveryService reads the .serviceconfig file to get version information, I'm not sure how else to get around it.

See this PR discussion for more details: https://github.com/ni/measurement-plugin-python/pull/907#discussion_r1769231913

AB#2863530

bkeryan commented 1 month ago

In addition to @nick-beer's suggestions, I think it would be reasonable to emit a DeprecationWarning:

if version:
    warnings.warn("The 'version' parameter is deprecated. Specify the version in the .serviceconfig file instead.", DeprecationWarning)