Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove per-property "inheritance" from cached objects #403

Closed
michaelcfanning opened this issue Apr 28, 2019 · 1 comment
Closed

Remove per-property "inheritance" from cached objects #403

michaelcfanning opened this issue Apr 28, 2019 · 1 comment
Labels

Comments

@michaelcfanning
Copy link
Contributor

michaelcfanning commented Apr 28, 2019

Addresses and logical locations have a parenting mechanism to provide for caching. Adding a second caching mechanism for these creates unnecessary complexity for consumers that doesn't provide clear value. If producers provide an index, they can recapitulate duplicative properties for purposes of readability but they must remain in sync with the run level version. And so we should remove index.

Even after removing logicalLocation.index and address.index, the complexity we refer to still applies to the other cached objects threadFlowLocation, webRequest, and webResponse.

Part of the problem is that we have never clearly distinguished between two concepts; we have used the term "caching" to refer to both:

  • Reuse: Multiple "local objects," for example multiple threadFlowLocation objects in result.codeFlows[0].threadFlows can reuse the same "cached object in run.threadFlowLocations by specifying their threadFlowLocation.index property.

  • Inheritance: Those "local objects" can inherit some properties from the "cached object", while at the same time specifying other properties locally.

It is the inheritance part that really complicates the SDK. Therefore, we propose to remove the inheritance mechanism. For the remaining cached objects threadFlowLocation, webRequest, and webResponse, we propose to require that either the index property or a set of local properties, but not both, be present.

UPDATE @lgolding 4/29/2019: We are restoring the index property on address and logicalLocation. So this is no longer a schema change (although it is still breaking).

@ghost ghost self-assigned this Apr 28, 2019
ghost pushed a commit that referenced this issue Apr 28, 2019
@ghost ghost added merged Changes merged into provisional draft. resolved-fixed and removed to-be-written labels Apr 28, 2019
@ghost ghost closed this as completed Apr 28, 2019
@ghost ghost changed the title Inline property overrides is broken Remove per-property "inheritance" from cached objects Apr 28, 2019
@ghost ghost removed the schema-todo label Apr 29, 2019
ghost pushed a commit that referenced this issue Apr 29, 2019
@ghost ghost reopened this Apr 29, 2019
@ghost
Copy link

ghost commented Apr 29, 2019

Sorry, reopened the wrong issue. Should have been #401.

@ghost ghost closed this as completed Apr 29, 2019
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant