Closed bhavesh37 closed 5 years ago
I think this looks good, but I'll ask Andy to review just to make sure. Good job Bhavesh
A few questions/observations:
This seems to be more than just the TCPInfo changes since it also contains configuration options related to the LS. Is it intentional to are these changes that got pulled in from another branch?
The changes to lib/perfSONAR_PS/Utils/LookupService.pm have the potential to break things. That is a class that may be used outside the toolkit bundle and should not require a configuration file. Instead the subroutine discover_lookup_services should be updated to accept a parameter with the location of the LookupService. The configuration file should be read by the calling class and the value for the activehosts URL passed to the subroutine call.
Per my second bullet, I looked closer at this and you should be able remove the configuration file read from lib/perfSONAR_PS/Utils/LookupService.pm and update the calls to is_host_registered()
in lib/perfSONAR_PS/NPToolkit/DataService/Host.pm to pass the value of $conf{activehosts}
. The %conf
hash already has the contents of that file so there is no need to read it again. For an example, see how it handles allow_internal_addresses
from web_admin.conf. For each call in Host.pm to is_host_registered()
you can do something like the following:
$is_registered = is_host_registered($external_address, $conf{activehosts});
You will then need to update the subroutines in lib/perfSONAR_PS/Utils/LookupService.pm to accept and pass through the optional activehosts parameter as needed. If the activehosts argument is not present when the subroutine is called, it should probably just fallback to the old behavior for backward compatibility.
Created separate branches for each toolkit issue/commit
Added TCP config method to show tcp info on UI and host JSON