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

Release 1.92.0 #2217

Merged
merged 30 commits into from
Mar 12, 2025
Merged
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
e41eeb1
simplifying parse_status logic
bistline Feb 12, 2025
527d961
Renaming test
bistline Feb 26, 2025
2d681a5
Removing all explicit Terra status checks
bistline Feb 26, 2025
50a8995
Removing more checks, test regression fixes
bistline Feb 26, 2025
0a4d04a
Merge pull request #2210 from broadinstitute/jb-parse-flags-logic
bistline Feb 27, 2025
93421b3
Adding API endpoint for submitting user-requested DE jobs
bistline Feb 27, 2025
ee68be6
Refactoring error handling, better messaging
bistline Mar 3, 2025
3c72458
DRYing request validation into DifferentialExpressionService
bistline Mar 3, 2025
fc85784
Fixing Swagger doc, test regressions
bistline Mar 3, 2025
22c1ead
Merge pull request #2211 from broadinstitute/jb-terra-status-lazy
bistline Mar 3, 2025
4578fc6
Fixing test regressions, updating test data re: OLS change on 252FHAN…
bistline Mar 3, 2025
7ad708d
Fixing test regression re: stale DE object
bistline Mar 3, 2025
a915c5a
Adding weekly quota for user-submitted DE jobs (default 5)
bistline Mar 4, 2025
a27746c
Mixpanel logging for quota exceptions
bistline Mar 4, 2025
12f666c
Merge pull request #2212 from broadinstitute/jb-pairwise-de-api
bistline Mar 5, 2025
47c8816
Fixing query logic & order of operations for AnnData DE jobs
bistline Mar 6, 2025
88dec72
Refactoring for updates, re-enabling tests
bistline Mar 6, 2025
07eab04
no-op change to force CI
bistline Mar 10, 2025
dfc2fdf
Adding weekday staging deployment at 8:15AM
bistline Mar 10, 2025
81db641
Test regressions, reverting find_library_prep
bistline Mar 10, 2025
b7948ce
Merge pull request #2213 from broadinstitute/jb-de-result-matrix-file…
bistline Mar 10, 2025
53a76d0
Merge pull request #2214 from broadinstitute/jb-nemo-api-update
bistline Mar 10, 2025
9a9b26a
adding scripts to source to help deal with scheduled instances
bistline Mar 10, 2025
b87c1d7
Bump rack from 2.2.11 to 2.2.13
dependabot[bot] Mar 11, 2025
9f878fd
Fixing whitespace, reverting schedule
bistline Mar 11, 2025
5777768
addressing PR comments
bistline Mar 11, 2025
691448b
Merge pull request #2216 from broadinstitute/jb-staging-deploy-schedule
bistline Mar 11, 2025
c01168a
Merge pull request #2215 from broadinstitute/dependabot/bundler/rack-…
bistline Mar 11, 2025
529b1ba
Bump @babel/runtime-corejs2 from 7.22.6 to 7.26.10
dependabot[bot] Mar 11, 2025
b32f723
Merge pull request #2218 from broadinstitute/dependabot/npm_and_yarn/…
bistline Mar 12, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
@@ -368,7 +368,7 @@ GEM
puma (5.6.9)
nio4r (~> 2.0)
racc (1.8.1)
rack (2.2.11)
rack (2.2.13)
rack-brotli (1.1.0)
brotli (>= 0.1.7)
rack (>= 1.4)
13 changes: 0 additions & 13 deletions app/controllers/analysis_configurations_controller.rb
Original file line number Diff line number Diff line change
@@ -3,7 +3,6 @@ class AnalysisConfigurationsController < ApplicationController
:submission_preview, :load_study_for_submission_preview,
:update_analysis_parameter]
before_action :set_analysis_parameter, only: [:update_analysis_parameter]
before_action :check_firecloud_status, only: [:new, :create]
before_action do
authenticate_user!
authenticate_admin
@@ -177,16 +176,4 @@ def analysis_parameter_params
:association_method, :association_data_type,
:_destroy])
end

def check_firecloud_status
unless ApplicationController.firecloud_client.services_available?(FireCloudClient::RAWLS_SERVICE)
alert = 'The Methods Repository is temporarily unavailable, so we cannot complete your request. Please try again later.'
respond_to do |format|
format.js {render js: "$('.modal').modal('hide'); alert('#{alert}')" and return}
format.html {redirect_to merge_default_redirect_params(studies_path, scpbr: params[:scpbr]),
alert: alert and return}
format.json {head 503}
end
end
end
end
153 changes: 151 additions & 2 deletions app/controllers/api/v1/site_controller.rb
Original file line number Diff line number Diff line change
@@ -2,8 +2,8 @@ module Api
module V1
class SiteController < ApiBaseController
before_action :set_current_api_user!
before_action :authenticate_api_user!, only: [:download_data, :stream_data, :get_study_analysis_config,
:submit_study_analysis, :get_study_submissions,
before_action :authenticate_api_user!, only: [:download_data, :stream_data, :submit_differential_expression,
:get_study_analysis_config, :submit_study_analysis, :get_study_submissions,
:get_study_submission, :sync_submission_outputs]
before_action :set_study, except: [:studies, :check_terra_tos_acceptance, :analyses, :get_analysis, :renew_user_access_token]
before_action :set_analysis_configuration, only: [:get_analysis, :get_study_analysis_config]
@@ -403,6 +403,151 @@ def stream_data
end
end

swagger_path '/site/studies/{accession}/differential_expression' do
operation :post do
key :tags, [
'Site'
]
key :summary, 'Submit a differential expression calculation request'
key :description, 'Request differential expression calculations for a given cluster/annotation in a study'
key :operationId, 'site_study_submit_differential_expression_path'
parameter do
key :name, :accession
key :in, :path
key :description, 'Accession of Study to use'
key :required, true
key :type, :string
end
parameter do
key :name, :de_job
key :type, :object
key :in, :body
schema do
property :cluster_name do
key :description, 'Name of cluster group. Use "_default" to use the default cluster'
key :required, true
key :type, :string
end
property :annotation_name do
key :description, 'Name of annotation'
key :required, true
key :type, :string
end
property :annotation_scope do
key :description, 'Scope of annotation. One of "study" or "cluster".'
key :type, :string
key :required, true
key :enum, Api::V1::Visualization::AnnotationsController::VALID_SCOPE_VALUES
end
property :de_type do
key :description, 'Type of differential expression analysis. Either "rest" (one-vs-rest) or "pairwise"'
key :type, :string
key :required, true
key :enum, %w[rest pairwise]
end
property :group1 do
key :description, 'First group for pairwise analysis (optional)'
key :type, :string
end
property :group2 do
key :description, 'Second group for pairwise analysis (optional)'
key :type, :string
end
end
end
response 204 do
key :description, 'Job successfully submitted'
end
response 401 do
key :description, ApiBaseController.unauthorized
end
response 403 do
key :description, ApiBaseController.forbidden('view study, study has author DE')
end
response 404 do
key :description, ApiBaseController.not_found(Study, 'Cluster', 'Annotation')
end
response 406 do
key :description, ApiBaseController.not_acceptable
end
response 409 do
key :description, "Results are processing or already exist"
end
response 410 do
key :description, ApiBaseController.resource_gone
end
response 422 do
key :description, "Job parameters failed validation"
end
response 429 do
key :description, 'Weekly user quota exceeded'
end
end
end

def submit_differential_expression
# disallow DE calculation requests for studies with author DE
if @study.differential_expression_results.where(is_author_de: true).any?
render json: {
error: 'User requests are disabled for this study as it contains author-supplied differential expression results'
}, status: 403 and return
end

# check user quota before proceeding
if DifferentialExpressionService.job_exceeds_quota?(current_api_user)
# minimal log props to help gauge overall user interest, as well as annotation/de types
log_props = {
studyAccession: @study.accession, annotationName: params[:annotation_name], de_type: params[:de_type]
}
MetricsService.log('quota-exceeded-de', log_props, current_api_user, request:)
current_quota = DifferentialExpressionService.get_weekly_user_quota
render json: { error: "You have exceeded your weekly quota of #{current_quota} requests" },
status: 429 and return
end

cluster_name = params[:cluster_name]
cluster = cluster_name == '_default' ? @study.default_cluster : @study.cluster_groups.by_name(cluster_name)
render json: { error: "Requested cluster #{cluster_name} not found" }, status: 404 and return if cluster.nil?

annotation_name = params[:annotation_name]
annotation_scope = params[:annotation_scope]
de_type = params[:de_type]
pairwise = de_type == 'pairwise'
group1 = params[:group1]
group2 = params[:group2]
annotation = AnnotationVizService.get_selected_annotation(
@study, cluster:, annot_name: annotation_name, annot_type: 'group', annot_scope: annotation_scope
)
render json: { error: 'No matching annotation found' }, status: 404 and return if annotation.nil?

de_params = { annotation_name:, annotation_scope:, de_type:, group1:, group2: }

# check if these results already exist
# for pairwise, also check if requested comparisons exist
result = DifferentialExpressionResult.find_by(
study: @study, cluster_group: cluster, annotation_name:, annotation_scope:, is_author_de: false
)
if result && (!pairwise || (pairwise && result.has_pairwise_comparison?(group1, group2)))
render json: { error: "Requested results already exist" }, status: 409 and return
end

begin
submitted = DifferentialExpressionService.run_differential_expression_job(
cluster, @study, current_api_user, **de_params
)
if submitted
DifferentialExpressionService.increment_user_quota(current_api_user)
head 204
else
# submitted: false here means that there is a matching running DE job
render json: { error: "Requested results are processing - please check back later" }, status: 409
end
rescue ArgumentError => e
# job parameters failed to validate
render json: { error: e.message}, status: 422 and return
end
end

swagger_path '/site/analyses' do
operation :get do
key :tags, [
@@ -1136,6 +1281,10 @@ def get_download_quota
end
end

def de_job_params
params.require(:de_job).permit(:cluster_name, :annotation_name, :annotation_scope, :de_type, :group1, :group2)
end

# update AnalysisSubmissions when loading study analysis tab
# will not backfill existing workflows to keep our submission history clean
def update_analysis_submission(submission)
2 changes: 0 additions & 2 deletions app/controllers/api/v1/studies_controller.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
module Api
module V1
class StudiesController < ApiBaseController
include Concerns::FireCloudStatus

def firecloud_independent_methods
# add file_info is essentially a more extensive 'show' method
[:index, :show, :file_info]
2 changes: 0 additions & 2 deletions app/controllers/api/v1/study_files_controller.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
module Api
module V1
class StudyFilesController < ApiBaseController
include Concerns::FireCloudStatus

before_action :authenticate_api_user!
before_action :set_study
before_action :check_study_edit_permission
2 changes: 0 additions & 2 deletions app/controllers/api/v1/study_shares_controller.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
module Api
module V1
class StudySharesController < ApiBaseController
include Concerns::FireCloudStatus

before_action :authenticate_api_user!
before_action :set_study
before_action :check_study_permission
2 changes: 1 addition & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
@@ -264,7 +264,7 @@ def verify_file_download_permissions(study)
# next check if downloads have been disabled by administrator, this will abort the download
# download links shouldn't be rendered in any case, this just catches someone doing a straight GET on a file
# also check if workspace google buckets are available
if !AdminConfiguration.firecloud_access_enabled? || !ApplicationController.firecloud_client.services_available?(FireCloudClient::BUCKETS_SERVICE)
if !AdminConfiguration.firecloud_access_enabled?
head 503 and return
end
end
14 changes: 0 additions & 14 deletions app/controllers/billing_projects_controller.rb
Original file line number Diff line number Diff line change
@@ -18,7 +18,6 @@ class BillingProjectsController < ApplicationController
respond_to :html, :js, :json
before_action :authenticate_user!
before_action :check_firecloud_registration, except: :access_request
before_action :check_firecloud_status, except: :access_request
before_action :create_firecloud_client, except: :access_request
before_action :check_project_permissions, except: [:index, :create, :access_request]
before_action :load_service_account, except: [:new_user, :create_user, :delete_user, :storage_estimate, :access_request]
@@ -214,18 +213,5 @@ def check_project_permissions
alert: "You do not have permission to perform that action. #{SCP_SUPPORT_EMAIL}" and return
end
end

# check on FireCloud API status and respond accordingly
def check_firecloud_status
unless ApplicationController.firecloud_client.services_available?(FireCloudClient::THURLOE_SERVICE)
alert = "Billing projects are temporarily unavailable, so we cannot complete your request. Please try again later. #{SCP_SUPPORT_EMAIL}"
respond_to do |format|
format.js {render js: "$('.modal').modal('hide'); alert('#{alert}')" and return}
format.html {redirect_to merge_default_redirect_params(site_path, scpbr: params[:scpbr]),
alert: alert and return}
format.json {head 503}
end
end
end
end

24 changes: 10 additions & 14 deletions app/controllers/profiles_controller.rb
Original file line number Diff line number Diff line change
@@ -19,22 +19,18 @@ def show
@study_shares = StudyShare.where(email: @user.email)
@studies = Study.where(user_id: @user.id)
@fire_cloud_profile = FireCloudProfile.new
# check if profile services are available
@profiles_available = ApplicationController.firecloud_client.services_available?(FireCloudClient::THURLOE_SERVICE)
if @profiles_available
begin
user_client = FireCloudClient.new(user: current_user, project: FireCloudClient::PORTAL_NAMESPACE)
profile = user_client.get_profile
profile['keyValuePairs'].each do |attribute|
if @fire_cloud_profile.respond_to?("#{attribute['key']}=")
@fire_cloud_profile.send("#{attribute['key']}=", attribute['value'])
end
begin
user_client = FireCloudClient.new(user: current_user, project: FireCloudClient::PORTAL_NAMESPACE)
profile = user_client.get_profile
profile['keyValuePairs'].each do |attribute|
if @fire_cloud_profile.respond_to?("#{attribute['key']}=")
@fire_cloud_profile.send("#{attribute['key']}=", attribute['value'])
end
rescue => e
ErrorTracker.report_exception(e, current_user, params)
MetricsService.report_error(e, request, current_user)
logger.info "#{Time.zone.now}: unable to retrieve FireCloud profile for #{current_user.email}: #{e.message}"
end
rescue => e
ErrorTracker.report_exception(e, current_user, params)
MetricsService.report_error(e, request, current_user)
logger.info "#{Time.zone.now}: unable to retrieve FireCloud profile for #{current_user.email}: #{e.message}"
end
end

54 changes: 6 additions & 48 deletions app/controllers/site_controller.rb
Original file line number Diff line number Diff line change
@@ -118,12 +118,7 @@ def update_study_settings
@cluster_annotations = ClusterVizService.load_cluster_group_annotations(@study, @cluster, current_user)
set_selected_annotation
end

# double check on download availability: first, check if administrator has disabled downloads
# then check if FireCloud is available and disable download links if either is true
@allow_downloads = ApplicationController.firecloud_client.services_available?(FireCloudClient::BUCKETS_SERVICE)
end
set_firecloud_permissions(@study.detached?)
set_study_permissions(@study.detached?)
set_study_default_options
set_study_download_options
@@ -156,7 +151,6 @@ def study
# double check on download availability: first, check if administrator has disabled downloads
# then check individual statuses to see what to enable/disable
# if the study is 'detached', then everything is set to false by default
set_firecloud_permissions(@study.detached?)
set_study_permissions(@study.detached?)
set_study_default_options
set_study_download_options
@@ -652,46 +646,19 @@ def set_workspace_samples
end
end

# check various firecloud statuses/permissions, but only if a study is not 'detached'
def set_firecloud_permissions(study_detached)
@allow_firecloud_access = false
@allow_downloads = false
@allow_edits = false
return if study_detached
begin
@allow_firecloud_access = AdminConfiguration.firecloud_access_enabled?
api_status = ApplicationController.firecloud_client.api_status
# reuse status object because firecloud_client.services_available? each makes a separate status call
# calling Hash#dig will gracefully handle any key lookup errors in case of a larger outage
if api_status.is_a?(Hash)
system_status = api_status['systems']
sam_ok = system_status.dig(FireCloudClient::SAM_SERVICE, 'ok') == true # do equality check in case 'ok' node isn't present
rawls_ok = system_status.dig(FireCloudClient::RAWLS_SERVICE, 'ok') == true
buckets_ok = system_status.dig(FireCloudClient::BUCKETS_SERVICE, 'ok') == true
@allow_downloads = buckets_ok
@allow_edits = sam_ok && rawls_ok
end
rescue => e
logger.error "Error checking FireCloud API status: #{e.class.name} -- #{e.message}"
ErrorTracker.report_exception(e, current_user, @study, { firecloud_status: api_status})
MetricsService.report_error(e, request, current_user, @study)
end
end

# set various study permissions based on the results of the above FC permissions
def set_study_permissions(study_detached)
@user_can_edit = false
@user_can_compute = false
@user_can_download = false
@user_embargoed = false
@allow_firecloud_access = AdminConfiguration.firecloud_access_enabled?

return if study_detached || !@allow_firecloud_access
begin
@user_can_edit = @study.can_edit?(current_user)
if @allow_downloads
@user_can_download = @user_can_edit ? true : @study.can_download?(current_user)
@user_embargoed = @user_can_edit ? false : @study.embargoed?(current_user)
end
@user_can_download = @user_can_edit ? true : @study.can_download?(current_user)
@user_embargoed = @user_can_edit ? false : @study.embargoed?(current_user)
rescue => e
logger.error "Error setting study permissions: #{e.class.name} -- #{e.message}"
ErrorTracker.report_exception(e, current_user, @study)
@@ -781,21 +748,12 @@ def check_view_permissions

# check compute permissions for study
def check_compute_permissions
if ApplicationController.firecloud_client.services_available?(FireCloudClient::SAM_SERVICE, FireCloudClient::RAWLS_SERVICE)
if !user_signed_in? || !@study.can_compute?(current_user)
@alert = "You do not have permission to perform that action. #{SCP_SUPPORT_EMAIL}"
respond_to do |format|
format.js {render action: :notice}
format.html {redirect_to merge_default_redirect_params(site_path, scpbr: params[:scpbr]), alert: @alert and return}
format.json {head 403}
end
end
else
@alert = "Compute services are currently unavailable - please check back later. #{SCP_SUPPORT_EMAIL}"
if !user_signed_in? || !@study.can_compute?(current_user)
@alert = "You do not have permission to perform that action. #{SCP_SUPPORT_EMAIL}"
respond_to do |format|
format.js {render action: :notice}
format.html {redirect_to merge_default_redirect_params(site_path, scpbr: params[:scpbr]), alert: @alert and return}
format.json {head 503}
format.json {head 403}
end
end
end
Loading