Added support for vmSettings' ETag #2294
Conversation
narrieta
requested review from
dhivyaganesan,
kevinclark19a,
larohra,
nagworld9 and
ZhidongPeng
as code owners
|
@jscalev FYI |
Codecov Report
@@ Coverage Diff @@
## fast-track #2294 +/- ##
==============================================
+ Coverage 70.60% 70.62% +0.01%
==============================================
Files 96 97 +1
Lines 13917 13939 +22
Branches 1986 1991 +5
==============================================
+ Hits 9826 9844 +18
- Misses 3656 3659 +3
- Partials 435 436 +1
Continue to review full report at Codecov.
|
| @@ -666,7 +670,7 @@ def fetch_manifest(self, version_uris, timeout_in_minutes=5, timeout_in_ms=0): | |||
| logger.verbose('The specified manifest URL is empty, ignored.') | |||
| continue | |||
|
|
|||
| direct_func = lambda: self.fetch(version.uri, max_retry=1) # pylint: disable=W0640 | |||
| direct_func = lambda: self.fetch(version.uri, max_retry=1)[0] # pylint: disable=W0640 | |||
| # NOTE: the host_func may be called after refreshing the goal state, be careful about any goal state data | |||
| # in the lambda. | |||
| host_func = lambda: self.fetch_manifest_through_host(version.uri) # pylint: disable=W0640 | |||
There was a problem hiding this comment.
There is a pitfall here that might be a good idea to document. direct_func and host_func both reference version.uri, but version is a mutating variable declared in a loop. Thus, after the loop, all the lambdas will use the last version.uri in version_uris_shuffled. See the pylint page for more info (just Ctrl+F for W0640). I believe this isn't an issue right now because we only access these lambdas from inside the loop, but any future code that somehow tries to export a particular direct_func/host_func would run into a pretty hard to debug error. Might be worth a code comment.
There was a problem hiding this comment.
We create a new closure in each iteration of the loop, so this is not an issue. I can add a comment explaining the suppression on a subsequent PR.
Use the ETag of the vmSettings API; update extensions goal state only if the ETag changed.