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

[HOLD] Rename queues to include replication_ and audit_ in the name #2185

Closed
wants to merge 3 commits into from

Conversation

aaron-collier
Copy link
Contributor

@aaron-collier aaron-collier commented Jan 24, 2023

Why was this change made? 🤔

Closes #2064 by renaming the replication job queues.
Closes #2056 by renaming the audit job queues.

When merging note the required shared configs PRs: https://github.com/sul-dlss/shared_configs/pulls/aaron-collier

How was this change tested? 🤨

⚡ ⚠ If this change has cross service impact, or if it changes code used internally for cloud replication, run integration test preassembly_image_accessioning_spec.rb against stage as it tests preservation, and/or test in stage environment, in addition to specs. The main classes relevant to replication are ZipmakerJob, DeliveryDispatcherJob, *DeliveryJob, ResultsRecorderJob, and DruidVersionZip; see here for overview diagram of replication pipeline.⚡

@aaron-collier aaron-collier changed the title [HOLD] Rename queues to include replication_ in the name Rename queues to include replication_ in the name Jan 24, 2023
@aaron-collier aaron-collier changed the title Rename queues to include replication_ in the name [HOLD] Rename queues to include replication_ in the name Jan 24, 2023
@aaron-collier aaron-collier changed the title [HOLD] Rename queues to include replication_ in the name Rename queues to include replication_ in the name Jan 24, 2023
@aaron-collier aaron-collier changed the title Rename queues to include replication_ in the name Rename queues to include replication_ and audit_ in the name Jan 25, 2023
@@ -18,7 +18,7 @@ module Replication
# the VM is updated to a new vesrion). Therefore, we receive the zip
# metadata from the process that actually created the zip file.
class DeliveryDispatcherJob < Replication::ZipPartJobBase
queue_as :zips_made
queue_as :replication_plexer
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we call delivery dispatcher? We're deprecating "plexer".

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah - that was from the ticket which was written before this class changed. So I agree with JLitt's request: replication_delivery_dispatcher.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do you feel about that suggestion @ndushay? I made these based on the ticket.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wrote the ticket and it was out of date. Sorry about that.

@aaron-collier aaron-collier changed the title Rename queues to include replication_ and audit_ in the name [HOLD] Rename queues to include replication_ and audit_ in the name Jan 25, 2023
@@ -18,7 +18,7 @@ module Replication
# the VM is updated to a new vesrion). Therefore, we receive the zip
# metadata from the process that actually created the zip file.
class DeliveryDispatcherJob < Replication::ZipPartJobBase
queue_as :zips_made
queue_as :replication_plexer
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah - that was from the ticket which was written before this class changed. So I agree with JLitt's request: replication_delivery_dispatcher.

@@ -6,7 +6,7 @@ module Replication
# Please update the configs for the various environments if it's renamed or moved.
# @note This name is slightly misleading, as this class solely deals with AWS US East 1 endpoint
class S3EastDeliveryJob < Replication::DeliveryJobBase
queue_as :s3_us_east_1_delivery
queue_as :replication_aws_us_east_1_delivery
Copy link
Contributor

Choose a reason for hiding this comment

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

again, follow the job name, not the ticket.

@@ -6,7 +6,7 @@ module Replication
# Please update the configs for the various environments if it's renamed or moved.
# @note This name is slightly misleading, as this class solely deals with AWS US West 2 endpoint
class S3WestDeliveryJob < Replication::DeliveryJobBase
queue_as :s3_us_west_2_delivery
queue_as :replication_aws_us_west_2_delivery
Copy link
Contributor

Choose a reason for hiding this comment

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

again, follow the job name, not the ticket.

Copy link
Member

@mjgiarlo mjgiarlo left a comment

Choose a reason for hiding this comment

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

Approve pending consideration of Justin's comment

Copy link
Contributor

@ndushay ndushay left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes, Aaron!

@ndushay
Copy link
Contributor

ndushay commented Feb 14, 2023

@aaron-collier just a gentle reminder this is still outstanding; perhaps next week when you're FR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants