Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Minimize Snapshot diff for Server Server Capabilities#7187

Merged
ocket8888 merged 4 commits intoapache:masterfrom
zrhoffman:minimize-snapshot-diff
Nov 18, 2022
Merged

Minimize Snapshot diff for Server Server Capabilities#7187
ocket8888 merged 4 commits intoapache:masterfrom
zrhoffman:minimize-snapshot-diff

Conversation

@zrhoffman
Copy link
Copy Markdown
Member

@zrhoffman zrhoffman commented Nov 14, 2022

This PR fixes #7113 by reordering the Server Server Capabilities in the new Snapshot to make Server Server capabilities occurring in both the old and new arrays have the same indices.

This PR also adds unit testing capabilities to Traffic Portal v1.


Which Traffic Control components are affected by this PR?

  • Traffic Portal

What is the best way to verify this PR?

  1. Build Traffic Portal

    npm run build
  2. Run the Traffic Portal unit tests:

    npm run test:ci

If this is a bugfix, which Traffic Control versions contained the bug?

  • 6.1.0
  • 7.0.2

PR submission checklist

@zrhoffman zrhoffman added bug something isn't working as intended Traffic Portal v1 related to Traffic Portal version 1 low impact affects only a small portion of a CDN, and cannot itself break one labels Nov 14, 2022
Comment thread .github/workflows/tp.integration.tests.yml
Copy link
Copy Markdown
Contributor

@rimashah25 rimashah25 left a comment

Choose a reason for hiding this comment

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

Code changes LGTM. Unit tests are passing on local using karma
Added test server capability to a server and took a snap shot. Diff looks as follows:
image
Added eas server capability to a server and took a snap shot. Diff looks as follows:
image

I do see that the index is at 1 instead of 0 seen in master branch. Also change type is seen as add instead of update.

@rimashah25
Copy link
Copy Markdown
Contributor

Can be merged once TP tests pass.

oldItemsNext = oldItemsIterator.next();
newItemsNext = newItemsIterator.next();
}
minimalDiffItems.push(...addedItems);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

fyi, there's a builtin for just concatenating two arrays without doing a spread operation into a push: Array.prototype.concat.

@ocket8888 ocket8888 merged commit 962e7b1 into apache:master Nov 18, 2022
@zrhoffman zrhoffman deleted the minimize-snapshot-diff branch November 18, 2022 17:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug something isn't working as intended low impact affects only a small portion of a CDN, and cannot itself break one Traffic Portal v1 related to Traffic Portal version 1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Assigning a new Server Server capability changes SSC order in the Snapshot for that Server

3 participants