-
Notifications
You must be signed in to change notification settings - Fork 41
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
Batch aggregation #4303
Merged
Merged
Batch aggregation #4303
Changes from 19 commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
883016a
Aggregation migration, model, spec, factory
zwolf 5f2076b
Client & spec
zwolf 6683ba5
Policy and spec
zwolf 0adb3f1
Schemas
zwolf 38e4efe
Update policy & schemas
zwolf d4067ec
Update status enum
zwolf 2d8f8e2
Add default to status migration
zwolf 7db71c9
Update serializer
zwolf 3a88a0c
Update factory
zwolf 03ffb51
Controller and spec
zwolf b3b7292
Fix migration
zwolf 9b8d423
Hound
zwolf a16c421
Hound again
zwolf ea565fb
Third round hound
zwolf f6440bd
Feed hound
zwolf 25e1843
Consistency
zwolf ec55ab9
Merge branch 'master' into batch-aggregation
zwolf 50f14c1
Add project_id to Aggregations to allow Doorkeeper to scope by project
zwolf 83be921
Merge branch 'batch-aggregation' of github.com:zooniverse/panoptes in…
zwolf ee47747
Clarify admin specs, add collab spec
zwolf 0271b4c
Remove ignored_columns
zwolf 2fccdb7
Add spec for failed service connections
zwolf 4f1d46c
Add serializer spec
zwolf a08dbd1
Implement #destroy, add error specs for collisions
zwolf d97e027
Aggregation documentation
zwolf b90bdcc
reorder spec
zwolf 419dc65
Merge branch 'master' into batch-aggregation
zwolf c350406
Merge branch 'batch-aggregation' of github.com:zooniverse/panoptes in…
zwolf b175f36
Fix hash alignment
zwolf cc1fbfe
Remove aggregation association from subject
zwolf 871a147
Remove user uniqueness constraint
zwolf 1923b00
Merge branch 'master' into batch-aggregation
zwolf 99cb844
MIgrate database, update structure.sql
zwolf 7009cf4
Resolve migrations
zwolf File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,19 +1,25 @@ | ||
# frozen_string_literal: true | ||
|
||
class Api::V1::AggregationsController < Api::ApiController | ||
include JsonApiController::PunditPolicy | ||
|
||
require_authentication :create, :update, scopes: [:project] | ||
resource_actions :create, :update, :show, :index | ||
require_authentication :index, :show, :update, :create, scopes: [:project] | ||
resource_actions :index, :show, :create, :update | ||
schema_type :json_schema | ||
before_action :filter_by_subject_set, only: :index | ||
|
||
private | ||
|
||
def filter_by_subject_set | ||
subject_set_ids = params.delete(:subject_set_id).try(:split, ',') | ||
unless subject_set_ids.blank? | ||
@controlled_resources = controlled_resources | ||
.joins(workflow: :subject_sets) | ||
.where(workflows: { subject_set_id: subject_set_ids } ) | ||
def create | ||
zwolf marked this conversation as resolved.
Show resolved
Hide resolved
zwolf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
workflow = Workflow.find(create_params['links']['workflow']) | ||
project_id = workflow.project.id | ||
create_params['links']['project'] = project_id | ||
response = AggregationClient.new.send_aggregation_request( | ||
project_id, | ||
workflow.id, | ||
create_params['links']['user'] | ||
) | ||
super do |agg| | ||
agg.update({ task_id: response.body[:task_id], status: 'pending' }) | ||
end | ||
rescue AggregationClient::ConnectionError | ||
json_api_render(:service_unavailable, response.body) | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,20 @@ | ||
# frozen_string_literal: true | ||
|
||
class Aggregation < ApplicationRecord | ||
|
||
belongs_to :workflow | ||
belongs_to :subject | ||
|
||
validates_presence_of :workflow, :subject, :aggregation | ||
validates_uniqueness_of :subject_id, scope: :workflow_id | ||
validate :aggregation, :workflow_version_present | ||
belongs_to :project | ||
belongs_to :user | ||
validates :project, presence: true | ||
validates :workflow, presence: true | ||
validates :user, presence: true | ||
validates :user_id, uniqueness: { scope: :workflow_id } | ||
|
||
private | ||
self.ignored_columns = %w[subject_id aggregation] | ||
zwolf marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
def workflow_version_present | ||
wv_key = :workflow_version | ||
if aggregation && !aggregation.symbolize_keys.has_key?(wv_key) | ||
errors.add(:aggregation, "must have #{wv_key} metadata") | ||
end | ||
end | ||
enum status: { | ||
created: 0, | ||
pending: 1, | ||
completed: 2, | ||
failed: 3 | ||
} | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,30 +1,22 @@ | ||
class AggregationPolicy < ApplicationPolicy | ||
class ReadScope < Scope | ||
# Allow access to public aggregations | ||
class Scope < Scope | ||
def resolve(action) | ||
updatable_parents = policy_for(Workflow).scope_for(:update) | ||
updatable_scope = scope.joins(:workflow).merge(updatable_parents) | ||
|
||
public_aggregations = scope.joins(:workflow).where("workflows.aggregation ->> 'public' = 'true'") | ||
Aggregation.union(updatable_scope, public_aggregations) | ||
end | ||
end | ||
|
||
class WriteScope < Scope | ||
def resolve(action) | ||
parent_scope = policy_for(Workflow).scope_for(action) | ||
parent_scope = policy_for(Workflow).scope_for(:update) | ||
scope.where(workflow_id: parent_scope.select(:id)) | ||
end | ||
end | ||
|
||
scope :index, :show, with: ReadScope | ||
scope :update, :destroy, :versions, :version, with: WriteScope | ||
|
||
def linkable_subjects | ||
policy_for(Subject).scope_for(:show) | ||
end | ||
scope :index, :show, :create, :update, :destroy, with: Scope | ||
|
||
def linkable_workflows | ||
policy_for(Workflow).scope_for(:update) | ||
end | ||
|
||
def linkable_projects | ||
policy_for(Project).scope_for(:update) | ||
end | ||
|
||
def linkable_users | ||
policy_for(User).scope_for(:update) | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,27 +1,19 @@ | ||
class AggregationUpdateSchema < JsonSchema | ||
schema do | ||
type "object" | ||
description "An Aggregation for a subject" | ||
required "aggregation", "links" | ||
type 'object' | ||
description 'An Aggregation for a workflow' | ||
additional_properties false | ||
|
||
property "aggregation" do | ||
type "object" | ||
property 'uuid' do | ||
type 'string' | ||
end | ||
|
||
property "links" do | ||
type "object" | ||
additional_properties false | ||
|
||
required "subject", "workflow" | ||
|
||
property "subject" do | ||
type "integer", "string" | ||
end | ||
property 'task_id' do | ||
type 'string' | ||
end | ||
|
||
property "workflow" do | ||
type "integer", "string" | ||
end | ||
property 'status' do | ||
type 'string' | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
# frozen_string_literal: true | ||
|
||
class AggregationClient | ||
class ConnectionError < StandardError; end | ||
class ResourceNotFound < ConnectionError; end | ||
class ServerError < ConnectionError; end | ||
|
||
attr_reader :connection | ||
|
||
def initialize(adapter=Faraday.default_adapter) | ||
@connection = connect!(adapter) | ||
@host ||= ENV.fetch('AGGREGATION_HOST', 'http://test.example.com') | ||
end | ||
|
||
def connect!(adapter) | ||
Faraday.new(@host, ssl: { verify: false }) do |faraday| | ||
faraday.request :json | ||
faraday.response :json, content_type: /\bjson$/ | ||
faraday.adapter(*adapter) | ||
end | ||
end | ||
|
||
def send_aggregation_request(project_id, workflow_id, user_id) | ||
params = { project_id: project_id, workflow_id: workflow_id, user_id: user_id } | ||
|
||
request(:post, '/run_aggregation') do |req| | ||
req.body = params.to_json | ||
end | ||
end | ||
|
||
private | ||
|
||
def request(http_method, params) | ||
response = connection.send(http_method, *params) do |req| | ||
req.headers['Accept'] = 'application/json' | ||
req.headers['Content-Type'] = 'application/json' | ||
req.options.timeout = 5 # open/read timeout in seconds | ||
req.options.open_timeout = 2 # connection open timeout in seconds | ||
yield req if block_given? | ||
end | ||
|
||
handle_response(response) | ||
rescue Faraday::TimeoutError, | ||
Faraday::ConnectionFailed => e | ||
raise ConnectionError, e.message | ||
end | ||
|
||
def handle_response(response) | ||
case response.status | ||
when 404 | ||
raise ResourceNotFound, status: response.status, body: response.body | ||
when 400..600 | ||
raise ServerError, response.body | ||
else | ||
response.body | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
# frozen_string_literal: true | ||
|
||
class RefactorAggregationModel < ActiveRecord::Migration[6.1] | ||
def up | ||
# delete existing unused columns | ||
safety_assured { remove_column :aggregations, :subject_id } | ||
safety_assured { remove_column :aggregations, :aggregation } | ||
|
||
# and the new aggregations columns | ||
add_column :aggregations, :project_id, :integer | ||
add_foreign_key :aggregations, :projects, column: :project_id, validate: false | ||
|
||
add_column :aggregations, :user_id, :integer | ||
add_foreign_key :aggregations, :users, column: :user_id, validate: false | ||
|
||
add_column :aggregations, :uuid, :string | ||
add_column :aggregations, :task_id, :string | ||
add_column :aggregations, :status, :integer, default: 0 | ||
end | ||
|
||
def down | ||
add_column :aggregations, :subject_id, :integer | ||
add_column :aggregations, :aggregation, :jsonb | ||
|
||
remove_column :aggregations, :user_id | ||
remove_column :aggregations, :project_id | ||
remove_column :aggregations, :uuid | ||
remove_column :aggregations, :task_id | ||
remove_column :aggregations, :status | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style.