search

HomecareHomebase / azure-bake

An Open Source Deployment Management Framework for DevOps and Developers
MIT License
9 stars 14 forks source link

coreutils.variable() behavior change #95

Closed bdschaap closed 5 years ago

bdschaap commented 5 years ago

The behavior of coreutils.variable() appears to have changed. It is now returning "" when calling the function for a Bake variable that does not exist. Before it would return null (or possibly undefined... not sure). This appears to have broken code in the storage ingredient's ConfigureDiagnosticSettings function such as this. let blobDiagnosticHourlyMetricsRetentionDays = await util.variable("blobDiagnosticHourlyMetricsRetentionDays") || 10

I should be able to adjust the code in ConfigureDiagnosticSettings but want to confirm that the behavior in coreutils.variable() shouldn't change back instead (but keep case insensitivity).

whilke commented 5 years ago

We always returned an empty string for variables we can't find. The bug is now on line 50 we return a new BakeVariable object for variables we can't lookup. Instead, line 50 should be:

return ""

Since it now returns a BakeVariable object your code will get an object assigned to your lvalue instead of 10.

bdschaap commented 5 years ago

Gotcha makes sense. Are you referring to line 68?

return new BakeVariable(def || "");

and it should be changed to this so the behavior is the same as before return "";

https://github.com/HomecareHomebase/azure-bake/blob/master/ingredient/ingredient-utils/src/functions.ts

bdschaap commented 5 years ago

Added fix in https://github.com/HomecareHomebase/azure-bake/pull/98