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

Remove Traffic Ops unnecessary WithCluster function#3174

Closed
rob05c wants to merge 1 commit intoapache:masterfrom
rob05c:to-remove-unnecessary-riak-with
Closed

Remove Traffic Ops unnecessary WithCluster function#3174
rob05c wants to merge 1 commit intoapache:masterfrom
rob05c:to-remove-unnecessary-riak-with

Conversation

@rob05c
Copy link
Member

@rob05c rob05c commented Dec 31, 2018

Removes the TO WithCluster function. Since the Riak cluster is now
a shared global variable, and never destructed, the WithCluster
abstraction serves no purpose, and just makes the code confusing.

WIP - I haven't tested yet.

What does this PR do?

Fixes #3346

Which TC components are affected by this PR?

  • Documentation
  • Grove
  • Traffic Analytics
  • Traffic Monitor
  • Traffic Ops
  • Traffic Ops ORT
  • Traffic Portal
  • Traffic Router
  • Traffic Stats
  • Traffic Vault
  • Other _________

What is the best way to verify this PR?

Check all that apply

  • This PR includes tests
  • This PR includes documentation updates
  • This PR includes an update to CHANGELOG.md
  • This PR includes all required license headers
  • This PR includes a database migration (ensure that migration sequence is correct)
  • This PR fixes a serious security flaw. Read more: www.apache.org/security

Removes the TO WithCluster function. Since the Riak cluster is now
a shared global variable, and never destructed, the WithCluster
abstraction serves no purpose, and just makes the code confusing.
@rob05c rob05c added Traffic Ops related to Traffic Ops WIP "Work-in-Progress" - do not merge! (use 'draft' pull requests from now on) tech debt rework due to choosing easy/limited solution labels Dec 31, 2018
@rob05c rob05c changed the title Remove TO unnecessary WithCluster Remove Traffic Ops unnecessary WithCluster function Dec 31, 2018
@asfgit
Copy link
Contributor

asfgit commented Dec 31, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/2999/
Test PASSed.

@ocket8888
Copy link
Contributor

The Traffic Vault system within Traffic Ops has undergone extensive refactor and changes since this was opened. Do these changes still make sense? Conflicts would also need to be resolved before support for Riak is removed entirely.

@rob05c
Copy link
Member Author

rob05c commented Jun 2, 2022

The withCluster function appears to still exist, is still called, and still serves no purpose:

https://github.com/apache/trafficcontrol/blob/1cfa82b0/traffic_ops/traffic_ops_golang/trafficvault/backends/riaksvc/riak_services.go#L387

https://github.com/apache/trafficcontrol/blob/0d84f2a8/traffic_ops/traffic_ops_golang/trafficvault/backends/riaksvc/dsutil.go#L54

Though as you say, that may not be true for very long.

@rob05c
Copy link
Member Author

rob05c commented Jun 2, 2022

This PR and/or resolving conflicts is the easy part, testing and making sure all Vault endpoints still work correctly is the bulk of the work.

Seems like that work probably isn't worth it, since the plan is to remove Riak support.

Since a separate issue exists for removing withCluster, I'll go ahead and close this, and we can close #3346 if and when the code ceases to exist.

And if the plan changes, or anyone thinks it's worth the work, it's easy enough to re-create this PR

@rob05c rob05c closed this Jun 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tech debt rework due to choosing easy/limited solution Traffic Ops related to Traffic Ops WIP "Work-in-Progress" - do not merge! (use 'draft' pull requests from now on)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove Traffic Ops riaksvc.WithCluster

3 participants