Register MSTransferor endpoint in REST layer #12241
Conversation
|
Jenkins results:
|
|
Jenkins results:
|
|
Jenkins results:
|
vkuznet
changed the title
There was a problem hiding this comment.
@vkuznet we are unfortunately adding more complexity and scattering functionalities in different places for these microservices, but I won't focus on that.
Looking at your implementation, I think this breaks the info API functionality for the ms-transferor.
For instance, this url /ms-transferor/data/info?request=dmapelli_ReReco_RunBlockWhite_Nvidia_test_v1_250124_095017_1803 is supposed to give me this answer [1], but I think with this patch, it will just forward such POST requests to the default post() method.
Can you please investigate this case? If needed, please apply your patch to cmsweb-testbed MSTransferor to finish this testing.
[1]
{"result": [
{
"wmcore_version": "2.3.8rc12",
"microservice_version": "2.3.8rc12",
"microservice": "MSManager",
"request": "dmapelli_ReReco_RunBlockWhite_Nvidia_test_v1_250124_095017_1803",
"transferDoc": {
"ConfigType": "TRANSFER",
"workflowName": "dmapelli_ReReco_RunBlockWhite_Nvidia_test_v1_250124_095017_1803",
"lastUpdate": 1737716023,
"transfers": [
{
"dataset": "/HLTPhysicsIsolatedBunch/Run2016H-v1/RAW",
"dataType": "primary",
"transferIDs": [
"ddc6ae4913984f77a6fd07bbe802100a"
],
"campaignName": "Agent239_Val",
"completion": [
0.0,
100.0
]
}
]
}
}]}
|
@amaltaro , thanks for brining this up, honestly I was not aware of such API/end-point. That said, what you shows is GET request while the changes I made is in POST method. But according to the code, the post method also look at This block of code tells me that when client make a POST request to My question is do you know a valid use-case for POST HTTP request to Finally, using these examples and test10 where this PR is deployed I see the following: as such the request goes to MSManager post API which routes it to MSTransferor. So far, I have this if flow: And, maybe we should reverse the order of if statements to Doing this way the payload with As I pointed out in #12240 the implicit structure of HTTP request handling in REST layer leads to many issues including one we are discussing now. Before making any changes I suggest to properly discuss and outline the HTTP request flow and write down all supported end-point calls which so far I do not posses. |
|
I think it is correct to assume that every single microservice support at least Given that each one of them might have a different behavior when serving that request, it is expected that each microservice will override the method itself, such that calls like I believe most (all?) of the time we call those APIs with the GET method, but we cannot know for certain unless we: Is it easy to see which endpoint the user is trying to access? If we can easily check that the user is accessing the |
|
Well, the only way to check calls to end-point is to look at cmsweb logs. As such it will never show all information by definition since:
Therefore, even though I can scan cmsweb logs to see /data/info in all logs I doubt it will definetly answer the question about its usage by all clients and all use-cases. What I pointing out that /data/info POST call seems weird to me since POST is suppose to create an object in service, what we create with /data/info? Therefore, I think it is safe to assume that /data/info should only be used by GET methods and remove completely
|
|
As you said, we cannot be certain about its usage. Anyhow, given this note in that post method "NOTE: the usage of this method requires further thought", I think we can do a small refactoring here and remove the I just wanted to point out that, if it wasn't for the |
|
ok, I removed |
|
Jenkins results:
|
GET requests with body don't seem to be a common usage, and this documentation suggests there is no clear definition on that: https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/GET @vkuznet please fix the unit tests as well. |
|
Jenkins results:
|
|
Jenkins results:
|
|
Jenkins results:
|
…services MS REST end-points
vkuznet
force-pushed
the
fix-issue-12040-fix4
branch
from
15c2b89 to
d1d50aa
Compare
|
Jenkins results:
|
vkuznet
force-pushed
the
fix-issue-12040-fix4
branch
from
d1d50aa to
1202158
Compare
|
Jenkins results:
|
|
The reported unit test failure |
Fixes #12040
Status
ready
Description
During integration tests I experimentally found that REST end-points are served by
Data.pymodule and delegate logic of API call toMSManager.py. Therefore, I added proper handling of REST end-point inData.pywhich calls MSManager post API.To better clarify HTTP request flow I provide full details how REST layer works in WM web framework.
The REST MicroService layer works in the following way:
def get,def postfunctions which are used to handle HTTP method of the request/data/ms-transferor/transferorend-point where/data/ms-transferoris FE redirect and/transferoris MS end-point is bound to instance ofDataclassdef postof Data class will handle HTTP request and callself.mgr.postmethod ofMSManager(hereself.mgrisMSManagerinstance)def postofMSManagerwill check the payload and route it to appropriate service API.Therefore WM REST web framework has three parts:
Datainstancedef postinDataclass which callsself.mgr.postto handle business logic of HTTP request.Is it backward compatible (if not, which system it affects?)
YES
Related PRs
#12155
External dependencies / deployment changes