[8.0] Separate handler class per service

[atsareg]

In the current Tornado HTTPS services implementation, Tornado is getting handler classes of each service which are used for creating handler objects to serve requests. The service configuration is made on the class level, for example, the database references. This lead to a problem that if two services, e.g. File Catalogs, use the same handler class, e.g. TornadoFileCatalogHandler, then the configuration of one service is overwritten by another one on the class level. This PR fixes the problem by creating separate derived handler class on the fly for each service which keeps the appropriate service configuration.
In particular, this PR should fix the problem with the MultiVOFileCatalog mentioned in #6769
There is also some simplification in the way how the full component name is evaluated leaving less freedom for developers which I think is the right thing. Only the service name as defined in the CS is taken

BEGINRELEASENOTES

*Core
FIX: HandlerManager - create separate class for each service handler
FIX: BaseRequestHandler - fullComponentName can be only defined in the configuration

ENDRELEASENOTES

[fstagni]

I can't fully evaluate the consequences of this PR, but I would suggest to (in this order)

• add these commits to [8.1] integration tests: HTTPs services by default #6769 to verify if the tests there are passing
• cherry-pick these commits in a PR for the integration branch and then run Thursday's hackathon with it.

would you do both? In case you need a quick-ish recipe on how to push to my PR, I normally do:

git fetch upstream pull/6769/head:localBranch-6769
git co localBranch-6769
...commits... (cherry-picks in your case)
git push git@github.com:fstagni/DIRAC.git localBranch-6769:81_tests_https

[chaen]

That is the sort of PR that terrifies me, because not only are the internals totally obscure, but it is absolutely not tested.
So I would definitely not feel comfortable having it in v8.0.

Moreover, I thought we agreed at the last BILD (or the one before) not to touch anymore that devilish framework beyond the trivial fix #6905

[atsareg]

@chaen: First, it is tested in the EGi test service. Second, the problem was introduced with the original https services that did not take this case into account. So, this is a fix to a very old problem which prevented exploring https services in the EGI environment, for example. Finally, this fixes this problem. The internals of this particular fix is quite clear (can not say so about the rest of this code indeed), so, I do not think it is introducing more entropy than before.

As for putting it into 8.0 - indeed this looks dangerous for the production branch. Even if it was tested in EGI these are less comprehensive tests than the certification. That's why the idea is to cherry-pick it into integration to test in the certification hackathon first

[chaen]

The internals of this particular fix is quite clear

Frankly not to me, but I trust you know what you are doing.

It seems indeed that a particular use case was overlooked at the beginning, but you could easily work around by running another tornado server in a separate process, like we do for the master CS for example.
yes, it's an operational fix, but it is way safer

[atsareg]

This changes are merged to integration and tested in the certification hackathon. Note that the actual change is one line

DIRAC/src/DIRAC/Core/Tornado/Server/HandlerManager.py

Line 116 in 2c35698

handler = type(

. Others are rather cosmetics. Further comments ?

[chrisburr]

Further comments ?

Do we have a fix for the impersonation? Until #6877 is understood I think it's a very bad idea to be running HTTPS services.

[chaen]

Further comments ?

Do we have a fix for the impersonation? Until #6877 is understood I think it's a very bad idea to be running HTTPS services.

As far as LHCb is concerned, there is not going to be a single HTTPs service deployed until there is a PR which

1. fixes it
2. comes with a test that fails before, passes after

But I can only speak for LHCb.

[fstagni]

Further comments ?

Do we have a fix for the impersonation? Until #6877 is understood I think it's a very bad idea to be running HTTPS services.

As far as LHCb is concerned, there is not going to be a single HTTPs service deployed until there is a PR which

1. fixes it
2. comes with a test that fails before, passes after

But I can only speak for LHCb.

We have yet to demonstrate that the current https service are to be blamed, as the ReportGenerator is a DISET service. I don't see this PR making anything bad.

[fstagni]

Can't see nothing wrong with this PR. Merging.