Fiware Headers

[c0c0n3]

PR #132 renamed the Fiware headers in the QL API spec:

1. fiwareService → fiware-Service
2. fiwareServicePath → fiware-ServicePath

While it's a good thing to align QL header names to Orion's, we also have to take API backward compatibility into account. To this end, we could:

1. Keep the existing names for a period of time with a deprecation warning.
2. Add the new names above and flag them as the proper header names to use.

that is, we could support both header names for a few releases and then ditch fiwareService and fiwareServicePath. Under the bonnet QL should be able to handle all four headers at the same time by just taking their content and store it in the corresponding Crate columns. If both new and old headers are specified at the same time in a request, the new should override the old.

[pooja1pathak]

Hi @c0c0n3 , I want contribute for this issue. Please assign it to me.

[c0c0n3]

@pooja1pathak awe, thanks! it's your baby now :-)

[pooja1pathak]

Hi @c0c0n3 , I have checked PR #132 and I have following queries:

1. In QL's code request.headers.get('fiware-service', None) and request.headers.get('fiware-servicepath', None) is used for Fiware-Service and Fiware-ServicePath respectively. I have checked previous versions and the same is used there also.
   As per my analysis request.headers.get('fiwareservice', None) and request.headers.get('fiwareservicepath', None) needs to be added to all files to support functionality as described in Fiware Headers #148 (comment). Please confirm my understanding.
2. would it be ok to show deprecation warning in Fiware headers description in API specification and on logs if deprecated Fiware headers are used?
3. Do we need to handle different combination of previous and new Fiware headers like:
   case-1: fiware-Service used with fiwareServicePath
   case-2: fiwareService used with fiware-ServicePath?

[c0c0n3]

Hi @pooja1pathak, PR #132 had to be reworked, what actually got implemented is PR #142---sorry I should've mentioned this before. In fact, I've just realised one thing that isn't clear from the issue description above is that the renaming of the headers got left out so the header names in the spec never changed, i.e. they're still called fiwareService and fiwareServicePath:

https://github.com/smartsdk/ngsi-timeseries-api/blob/master/specification/quantumleap.yml#L123

Again, sorry for the confusion. On to your questions.

1. In QL's code request.headers.get('fiware-service', None) and request.headers.get('fiware-servicepath', None) is used for Fiware-Service and Fiware-ServicePath respectively. I have checked previous versions and the same is used there also.

Whoops, then the spec has always been out of synch w/r/t the implementation I suppose! If that's the case, can you please check what happens if you curl a request with fiwareService and fiwareServicePath headers? Do they get ignored? If so we don't need to worry about backward compatibility since clients out there must've been using headers named: fiware-service and fiware-servicepath---notice the dash! Also it looks like we've always used the dashed version of the header names in our test cases too, see e.g.

https://github.com/smartsdk/ngsi-timeseries-api/blob/master/src/reporter/tests/test_multitenancy.py

@chicco785 can you please check what the Portal does? i.e. which set of header are we using there

spec = { fiwareService, fiwareServicePath }
    or
actual = { fiware-service, fiware-servicepath }

I suspect we're using actual across the board---this is what we do in StoryWine too. If it turns out this is true, then I suggest we just rename the headers in the spec and do nothing else.

[c0c0n3]

@pooja1pathak re: your other points above, if we just rename the headers in the spec, then please ignore my comments below since they won't be relevant anymore!

As per my analysis request.headers.get('fiwareservice', None) and request.headers.get('fiwareservicepath', None) needs to be added to all files

yep, that's correct, but I'd factor that logic out in a single function if possible and add the dashed header versions too which are currently scattered all over the show. It'd be great, if you could do this refactoring for us :-)

would it be ok to show deprecation warning in Fiware headers description in API specification and on logs if deprecated Fiware headers are used?

Perfect, thanks!

Do we need to handle different combination of previous and new Fiware headers

Yep, we'd need to be able to handle all possible combinations...

[chicco785]

@chicco785 can you please check what the Portal does? i.e. which set of header are we using there

@Cerfoglg is the best person to answer

[pooja1pathak]

Hi @c0c0n3 @chicco785, Any update on this?

As per my analysis we have used fiware-service and fiware-servicepath in code. fiwareService and fiwareServicePath is only used in API Specification. So, no backward compatibility will be needed.

I suspect we're using actual across the board---this is what we do in StoryWine too. If it turns out this is true, then I suggest we just rename the headers in the spec and do nothing else.

I think just renaming the headers in the spec will do the thing. Please confirm so that I can raise PR and close this issue.

[c0c0n3]

Hi @pooja1pathak,

We haven't had time to do our due diligence yet but thank you so much for the analysis and yes, if that's the case, I agree with you, all we need to do is rename the headers. Did you test it though? i.e. did you check what happens if you POST a notify request with fiwareService and fiwareServicePath headers? Do they get ignored and the payload gets stored in the default namespace in Crate? We need to test the same scenario for the Timescale backend too.

We'll run a sanity check for our other components next week, but I suppose for the time being you can go ahead with the PR assuming just a rename will do. If you could please add test cases for the two scenarios I mentioned earlier, that'd be great!

Thank you sooooo much!!!

[pooja1pathak]

Hi @c0c0n3 ,

Did you test it though? i.e. did you check what happens if you POST a notify request with fiwareService and fiwareServicePath headers?

Yes I have tested it for all the APIs mentioned in specification.

As per my observation and test results, code does not handle fiwareService/fiwareServicePath. Only fiware-Service/fiware-ServicePath (with '-' sign) is considered.

Do they get ignored and the payload gets stored in the default namespace in Crate?

Yes your understanding is correct. fiwareService and fiwareServicePath are ignored and payload gets stored in the default namespace in Crate.

[c0c0n3]

@pooja1pathak

have tested it for all the APIs ...

good stuff!!! thanks for this!

fiwareService and fiwareServicePath are ignored

excellent, thanks for checking!!!

[c0c0n3]

closed by #283