-
Notifications
You must be signed in to change notification settings - Fork 2
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
Adds publish robot. #5198
Adds publish robot. #5198
Conversation
@@ -21,6 +22,10 @@ gem 'purl_fetcher-client', '~> 2.1' | |||
gem 'stanford-mods' # for indexing | |||
gem 'sul_orcid_client', '~> 0.3' | |||
|
|||
source 'https://gems.contribsys.com/' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Robots use Sidekiq Pro.
@@ -107,7 +107,7 @@ def accession | |||
def publish | |||
result = BackgroundJobResult.create | |||
EventFactory.create(druid: params[:id], event_type: 'publish_request_received', data: { background_job_result_id: result.id }) | |||
PublishJob.set(queue: publish_queue).perform_later(druid: params[:id], background_job_result: result, workflow: params[:workflow]) | |||
PublishJob.set(queue: publish_queue).perform_later(druid: params[:id], background_job_result: result) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't get rid of this endpoint since it is invoked by DSA; however, it no longer needs to be concerned with workflows.
workflow:, | ||
workflow_process: workflow_process_for(workflow), | ||
output: { errors: [{ title: 'Publishing error', detail: 'Cannot publish an admin policy' }] }) | ||
background_job_result.output = { errors: [{ title: 'Publishing error', detail: 'Cannot publish an admin policy' }] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is no longer updating workflows, LogFailureJob / LogSuccessJob are no longer necessary.
end | ||
|
||
Publish::MetadataTransferService.publish(druid:, user_version:, workflow:) | ||
Publish::MetadataTransferService.publish(druid:, user_version:) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to pass on workflow to MetadataTransferService.
@@ -0,0 +1,21 @@ | |||
# frozen_string_literal: true | |||
|
|||
module Robots |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well isn't this robot simple.
@@ -0,0 +1,18 @@ | |||
# frozen_string_literal: true | |||
|
|||
module Robots |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Override some methods from LyberCore::Robot since this is running in DSA.
# @param [String] workflow (optional) the workflow used for reporting back status to (defaults to 'accessionWF') | ||
def self.publish(druid:, user_version: nil, workflow: 'accessionWF') | ||
new(druid:, workflow:, user_version:).publish | ||
def self.publish(druid:, user_version: nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MetadataTransferService no longer needs to know about workflows.
# Add the following to a sidekiq.yml to have it handle robot jobs. | ||
# :labels: | ||
# - robot | ||
if config[:labels].include?('robot') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This configures Sidekiq server differently whether running robots or normal DSA jobs.
@@ -128,15 +128,6 @@ paths: | |||
required: true | |||
schema: | |||
$ref: "#/components/schemas/Druid" | |||
- name: workflow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer needed in API.
37029a2
to
53635c2
Compare
refs #5197
Why was this change made? 🤔
Simplification. See ticket.
How was this change tested? 🤨
Unit, QA.
⚡ ⚠ If this change has cross service impact, including data writes to shared file systems, run integration tests and/or test in [stage|qa] environment, in addition to specs. ⚡