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

[Linear cache] Increase mutualization between delta and sotw in linear cache to have behavior driven logic instead #14

Merged
merged 2 commits into from
Feb 23, 2024

Conversation

valerian-roche
Copy link
Collaborator

Following the addition of stable version support in sotw, the remaining differences of behavior between sotw and delta are:

  • on first request, check the returned version in sotw if activating stable version. Also compute the version differently
  • in some cases, return full state in sotw. In other case don't return if the only change is deletion
  • compute the version differently

In this context, the code can now be mostly merged, apart from the type differences on requests and responses (which I think can be reduced to none soon, but requires more rework on simple side and would be somewhat backward incompatible).
In this PR, the linear cache no longer treats sotw and delta watches differently. They are tracked in the same maps and the changes and responses are computed in a common method. Differences in behavior (e.g. full state or which version to use) is now a functional attribute of the watch itself.
There are two main parts not yet fully merged:

  • changing the version in sotw when using stable versions. The requirement to prepend cache version prefix makes it harder to abstract
  • defining whether a change only removing resources should trigger a response. This could be made a functional flag but given its intersection with fullStateResponses seemed fragile

@valerian-roche valerian-roche force-pushed the vr/linear-sotw-stable-versions branch from 16f4615 to ba5d383 Compare February 15, 2024 00:59
@valerian-roche valerian-roche marked this pull request as ready for review February 15, 2024 01:00
@valerian-roche valerian-roche force-pushed the vr/linear-sotw-stable-versions branch from ba5d383 to 71686da Compare February 16, 2024 19:51
@valerian-roche valerian-roche force-pushed the vr/linear-sotw-stable-versions branch 2 times, most recently from 11e1362 to 4c322a7 Compare February 16, 2024 20:22
@valerian-roche valerian-roche force-pushed the vr/linear-sotw-stable-versions branch from 4c322a7 to 8bb7c6b Compare February 16, 2024 20:54
…r cache to have behavior driven logic instead

Signed-off-by: Valerian Roche <valerian.roche@datadoghq.com>
@valerian-roche valerian-roche changed the base branch from vr/linear-sotw-stable-versions to dd/sotw-fixes February 18, 2024 02:05
Copy link

@atollena atollena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only have some very minor comments, this is a nice cleanup.

pkg/cache/v3/cache.go Outdated Show resolved Hide resolved
pkg/cache/v3/cache.go Outdated Show resolved Hide resolved
pkg/cache/v3/cache.go Show resolved Hide resolved
pkg/cache/v3/cache.go Outdated Show resolved Hide resolved
pkg/cache/v3/status.go Outdated Show resolved Hide resolved
pkg/cache/v3/status.go Outdated Show resolved Hide resolved
pkg/cache/v3/linear.go Outdated Show resolved Hide resolved
pkg/cache/v3/linear.go Outdated Show resolved Hide resolved
Signed-off-by: Valerian Roche <valerian.roche@datadoghq.com>
@valerian-roche valerian-roche merged commit 443e480 into dd/sotw-fixes Feb 23, 2024
4 checks passed
valerian-roche added a commit that referenced this pull request Feb 23, 2024
…r cache to have behavior driven logic instead (#14)

Following the addition of stable version support in sotw, the remaining differences of behavior between sotw and delta are:
 - on first request, check the returned version in sotw if activating stable version. Also compute the version differently
 - in some cases, return full state in sotw. In other case don't return if the only change is deletion
 - compute the version differently

In this context, the code can now be mostly merged, apart from the type differences on requests and responses (which I think can be reduced to none soon, but requires more rework on simple side and would be somewhat backward incompatible).
In this PR, the linear cache no longer treats sotw and delta watches differently. They are tracked in the same maps and the changes and responses are computed in a common method. Differences in behavior (e.g. full state or which version to use) is now a functional attribute of the watch itself.
There are two main parts not yet fully merged:
 - changing the version in sotw when using stable versions. The requirement to prepend cache version prefix makes it harder to abstract
 - defining whether a change only removing resources should trigger a response. This could be made a functional flag but given its intersection with `fullStateResponses` seemed fragile

Signed-off-by: Valerian Roche <valerian.roche@datadoghq.com>
zhiyanfoo pushed a commit that referenced this pull request Apr 10, 2024
…r cache to have behavior driven logic instead (#14)

Following the addition of stable version support in sotw, the remaining differences of behavior between sotw and delta are:
 - on first request, check the returned version in sotw if activating stable version. Also compute the version differently
 - in some cases, return full state in sotw. In other case don't return if the only change is deletion
 - compute the version differently

In this context, the code can now be mostly merged, apart from the type differences on requests and responses (which I think can be reduced to none soon, but requires more rework on simple side and would be somewhat backward incompatible).
In this PR, the linear cache no longer treats sotw and delta watches differently. They are tracked in the same maps and the changes and responses are computed in a common method. Differences in behavior (e.g. full state or which version to use) is now a functional attribute of the watch itself.
There are two main parts not yet fully merged:
 - changing the version in sotw when using stable versions. The requirement to prepend cache version prefix makes it harder to abstract
 - defining whether a change only removing resources should trigger a response. This could be made a functional flag but given its intersection with `fullStateResponses` seemed fragile

Signed-off-by: Valerian Roche <valerian.roche@datadoghq.com>
valerian-roche added a commit that referenced this pull request Apr 10, 2024
…r cache to have behavior driven logic instead (#14)

Following the addition of stable version support in sotw, the remaining differences of behavior between sotw and delta are:
 - on first request, check the returned version in sotw if activating stable version. Also compute the version differently
 - in some cases, return full state in sotw. In other case don't return if the only change is deletion
 - compute the version differently

In this context, the code can now be mostly merged, apart from the type differences on requests and responses (which I think can be reduced to none soon, but requires more rework on simple side and would be somewhat backward incompatible).
In this PR, the linear cache no longer treats sotw and delta watches differently. They are tracked in the same maps and the changes and responses are computed in a common method. Differences in behavior (e.g. full state or which version to use) is now a functional attribute of the watch itself.
There are two main parts not yet fully merged:
 - changing the version in sotw when using stable versions. The requirement to prepend cache version prefix makes it harder to abstract
 - defining whether a change only removing resources should trigger a response. This could be made a functional flag but given its intersection with `fullStateResponses` seemed fragile

Signed-off-by: Valerian Roche <valerian.roche@datadoghq.com>
valerian-roche added a commit that referenced this pull request May 9, 2024
…r cache to have behavior driven logic instead (#14)

Following the addition of stable version support in sotw, the remaining differences of behavior between sotw and delta are:
 - on first request, check the returned version in sotw if activating stable version. Also compute the version differently
 - in some cases, return full state in sotw. In other case don't return if the only change is deletion
 - compute the version differently

In this context, the code can now be mostly merged, apart from the type differences on requests and responses (which I think can be reduced to none soon, but requires more rework on simple side and would be somewhat backward incompatible).
In this PR, the linear cache no longer treats sotw and delta watches differently. They are tracked in the same maps and the changes and responses are computed in a common method. Differences in behavior (e.g. full state or which version to use) is now a functional attribute of the watch itself.
There are two main parts not yet fully merged:
 - changing the version in sotw when using stable versions. The requirement to prepend cache version prefix makes it harder to abstract
 - defining whether a change only removing resources should trigger a response. This could be made a functional flag but given its intersection with `fullStateResponses` seemed fragile

Signed-off-by: Valerian Roche <valerian.roche@datadoghq.com>
valerian-roche added a commit that referenced this pull request Aug 8, 2024
…r cache to have behavior driven logic instead (#14)

Following the addition of stable version support in sotw, the remaining differences of behavior between sotw and delta are:
 - on first request, check the returned version in sotw if activating stable version. Also compute the version differently
 - in some cases, return full state in sotw. In other case don't return if the only change is deletion
 - compute the version differently

In this context, the code can now be mostly merged, apart from the type differences on requests and responses (which I think can be reduced to none soon, but requires more rework on simple side and would be somewhat backward incompatible).
In this PR, the linear cache no longer treats sotw and delta watches differently. They are tracked in the same maps and the changes and responses are computed in a common method. Differences in behavior (e.g. full state or which version to use) is now a functional attribute of the watch itself.
There are two main parts not yet fully merged:
 - changing the version in sotw when using stable versions. The requirement to prepend cache version prefix makes it harder to abstract
 - defining whether a change only removing resources should trigger a response. This could be made a functional flag but given its intersection with `fullStateResponses` seemed fragile

Signed-off-by: Valerian Roche <valerian.roche@datadoghq.com>
shamdor pushed a commit that referenced this pull request Sep 23, 2024
…r cache to have behavior driven logic instead (#14)

Following the addition of stable version support in sotw, the remaining differences of behavior between sotw and delta are:
 - on first request, check the returned version in sotw if activating stable version. Also compute the version differently
 - in some cases, return full state in sotw. In other case don't return if the only change is deletion
 - compute the version differently

In this context, the code can now be mostly merged, apart from the type differences on requests and responses (which I think can be reduced to none soon, but requires more rework on simple side and would be somewhat backward incompatible).
In this PR, the linear cache no longer treats sotw and delta watches differently. They are tracked in the same maps and the changes and responses are computed in a common method. Differences in behavior (e.g. full state or which version to use) is now a functional attribute of the watch itself.
There are two main parts not yet fully merged:
 - changing the version in sotw when using stable versions. The requirement to prepend cache version prefix makes it harder to abstract
 - defining whether a change only removing resources should trigger a response. This could be made a functional flag but given its intersection with `fullStateResponses` seemed fragile

Signed-off-by: Valerian Roche <valerian.roche@datadoghq.com>
wdauchy pushed a commit that referenced this pull request Oct 25, 2024
…r cache to have behavior driven logic instead (#14)

Following the addition of stable version support in sotw, the remaining differences of behavior between sotw and delta are:
 - on first request, check the returned version in sotw if activating stable version. Also compute the version differently
 - in some cases, return full state in sotw. In other case don't return if the only change is deletion
 - compute the version differently

In this context, the code can now be mostly merged, apart from the type differences on requests and responses (which I think can be reduced to none soon, but requires more rework on simple side and would be somewhat backward incompatible).
In this PR, the linear cache no longer treats sotw and delta watches differently. They are tracked in the same maps and the changes and responses are computed in a common method. Differences in behavior (e.g. full state or which version to use) is now a functional attribute of the watch itself.
There are two main parts not yet fully merged:
 - changing the version in sotw when using stable versions. The requirement to prepend cache version prefix makes it harder to abstract
 - defining whether a change only removing resources should trigger a response. This could be made a functional flag but given its intersection with `fullStateResponses` seemed fragile

Signed-off-by: Valerian Roche <valerian.roche@datadoghq.com>
wdauchy pushed a commit that referenced this pull request Oct 25, 2024
…r cache to have behavior driven logic instead (#14)

Following the addition of stable version support in sotw, the remaining differences of behavior between sotw and delta are:
 - on first request, check the returned version in sotw if activating stable version. Also compute the version differently
 - in some cases, return full state in sotw. In other case don't return if the only change is deletion
 - compute the version differently

In this context, the code can now be mostly merged, apart from the type differences on requests and responses (which I think can be reduced to none soon, but requires more rework on simple side and would be somewhat backward incompatible).
In this PR, the linear cache no longer treats sotw and delta watches differently. They are tracked in the same maps and the changes and responses are computed in a common method. Differences in behavior (e.g. full state or which version to use) is now a functional attribute of the watch itself.
There are two main parts not yet fully merged:
 - changing the version in sotw when using stable versions. The requirement to prepend cache version prefix makes it harder to abstract
 - defining whether a change only removing resources should trigger a response. This could be made a functional flag but given its intersection with `fullStateResponses` seemed fragile

Signed-off-by: Valerian Roche <valerian.roche@datadoghq.com>
wdauchy pushed a commit that referenced this pull request Oct 25, 2024
…r cache to have behavior driven logic instead (#14)

Following the addition of stable version support in sotw, the remaining differences of behavior between sotw and delta are:
 - on first request, check the returned version in sotw if activating stable version. Also compute the version differently
 - in some cases, return full state in sotw. In other case don't return if the only change is deletion
 - compute the version differently

In this context, the code can now be mostly merged, apart from the type differences on requests and responses (which I think can be reduced to none soon, but requires more rework on simple side and would be somewhat backward incompatible).
In this PR, the linear cache no longer treats sotw and delta watches differently. They are tracked in the same maps and the changes and responses are computed in a common method. Differences in behavior (e.g. full state or which version to use) is now a functional attribute of the watch itself.
There are two main parts not yet fully merged:
 - changing the version in sotw when using stable versions. The requirement to prepend cache version prefix makes it harder to abstract
 - defining whether a change only removing resources should trigger a response. This could be made a functional flag but given its intersection with `fullStateResponses` seemed fragile

Signed-off-by: Valerian Roche <valerian.roche@datadoghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants