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

refactor(*): Split redis code to its own module #2175

Merged
merged 1 commit into from
Apr 24, 2018

Conversation

robzienert
Copy link
Member

Step one of v2 persistence. Functionally, nothing in this PR has changed. This PR breaks out all Redis code into its own orca-redis module:

  • Introduced NotificationClusterLock interface so our NotificationPollingAgents can have a locking implementation that may not necessarily be Redis. Also added RedisNotificationClusterLock which is functionally the same as what was there previously.
  • Moved all Redis-related code to orca-redis. This includes the migrators that were in orca-front50 (strong doubts that they belonged in that module to begin with).
  • Removed Redis dependency on in orca-queue.
  • Moved all Redis-related config into single package.
  • Added orca-core-tck, which includes AbstractExecutionRepositoryTck.

Things to cleanup after this:

  • OldPipelineCleanup...Agent and TopApplicationExecutionCleanup...Agent were moved into orca-redis. Non-ideal. Will refactor these to be storage impl-agnostic in a separate PR.

Other things of note:

  • I couldn't remove Rx from orca-core because ExecutionRepository includes it as part of its API. Womp womp. From the looks, that's the only spot, though.
  • Next steps will be a PR to get orca-sql in place & baseline Spring config to run Redis vs SQL (and also Redis & SQL for red/black).

@robfletcher
Copy link
Contributor

WRT the agents, it doesn’t necessarily feel wrong to include them in orca-redis as who knows if another storage implementation would even need them.

@robzienert
Copy link
Member Author

Ah, yes. I just mean I want to move them back to core without redis (if it makes sense to do so). I think they'll be valuable regardless of the persistence

@robzienert robzienert force-pushed the v2-persist branch 2 times, most recently from 323420f to bb452ea Compare April 24, 2018 17:16
Copy link
Contributor

@robfletcher robfletcher left a comment

Choose a reason for hiding this comment

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

I like this

}

Pool<Jedis> jedisPool = embeddedRedis.pool

Copy link
Contributor

Choose a reason for hiding this comment

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

Holy… why was this…?

Copy link
Member Author

Choose a reason for hiding this comment

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

No idea. Haha.

@robzienert robzienert merged commit 8b48a51 into spinnaker:master Apr 24, 2018
@robzienert robzienert deleted the v2-persist branch April 24, 2018 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants