feat(PollingConfigManager): Implemented caching headers in PollingConfigManager. #189
Conversation
yasirfolio3
changed the title
| } | ||
|
|
||
| if code == 304 { | ||
| cmLogger.Debug("The datafile was not modified and won't be downloaded again") |
There was a problem hiding this comment.
if we want to go this way, I am not sure if the logic :
if projectConfig.GetRevision() == previousRevision {
cmLogger.Debug(fmt.Sprintf("No datafile updates. Current revision number: %s", cm.projectConfig.GetRevision()))
closeMutex(nil)
return
}
is still needed. It is saying the same thing. I am not even sure if there is a real case scenario for that logic to be executed.
There was a problem hiding this comment.
What about if anyone is providing URL template (server) which doesn't provide status 304? but keeps the datafile same?
There was a problem hiding this comment.
are you saying that somebody would try to reimplement requester ? Our default requester is using http libs and I think you will always get Last Modified header. @mikeng13 , what do you think ?
There was a problem hiding this comment.
no am saying, like in FSC we use our datafile server, if anyone using like that for testing their own stuff then it may cause loading same projectconfig everytime.
pawels-optimizely
left a comment
pawels-optimizely
left a comment
There was a problem hiding this comment.
please look at my comment.
mikeproeng37
changed the title
pawels-optimizely
left a comment
pawels-optimizely
left a comment
There was a problem hiding this comment.
I would still want @mikeng13 to take a look at it.
pkg/config/polling_manager.go
Outdated
| return | ||
| } | ||
|
|
||
| if code == 304 { |
There was a problem hiding this comment.
you can use http.StatusNotModified instead of 304 ( it will be clearer)
There was a problem hiding this comment.
Updated all previously hardcoded status codes too.
There was a problem hiding this comment.
looks good, tested the code with datafile changes.
mikeproeng37
left a comment
mikeproeng37
left a comment
There was a problem hiding this comment.
Code looks fine to me. Are we able to run this against the DFM FSC test cases @yasirfolio3 or @msohailhussain
pkg/config/polling_manager.go
Outdated
| return | ||
| } | ||
|
|
||
| if code == 304 { |
|
@mikeng13 no we can't run FSC DFM right now. Steps definition to be added for that and also need to run datafile server (travis related change). |
Last-Modified.