-
Notifications
You must be signed in to change notification settings - Fork 53
Asychronous import and updates when applying Fair Data Station rdf #2210
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
base: main
Are you sure you want to change the base?
Conversation
stuzart
commented
Jun 9, 2025
- fix for Asynchronous import and updates from FDS #2197
moved to a common function and out of the inline js other than a call to the function
…gation needs to be manageable #2197 updated validation and tests accordingly
if import, needs a project. If update, needs an investigation
can't use references because of mismatch on the table ids being int
investigation_external_identifier: fair_data_station_inv.external_id, | ||
purpose: :update, content_blob: content_blob | ||
) | ||
fair_data_station_upload.save! |
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 would throw a 500 error if the upload failed validation for whatever reason
Investigation.transaction do | ||
@investigation = Seek::FairDataStation::Writer.new.update_isa(@investigation, data_station_inv, current_person, @investigation.projects, @investigation.policy) | ||
@investigation.save! | ||
in_progress = FairDataStationUpload.matching_updates_in_progress(@investigation, fair_data_station_inv.external_id) |
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.
Would this be better as a uniqueness validation?
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.
Very similar to app/views/investigations/_fair_data_station_update_status.html.erb, could be DRYed up maybe
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.
yes, also very similar to the sample extraction, persistence, sample type template creation, and data file unzipping.
|
||
investigation = Seek::FairDataStation::Writer.new.construct_isa(fair_data_station_inv, person, [project], policy) | ||
User.with_current_user(person) do | ||
investigation.save! |
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.
Consider checking investigation.valid? and logging more specific errors before save! The same thing can apply to the update job.
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.
True, I concidered this but it decided it is out of scope for this task. It does record the validation error in the task, and at one point I displayed it, but decided it was too cryptic. You'll have noticed that it said "Studies are invalid" when this is not the case, and infact is caused by invalid samples. What it needs is to iterate over all the items and generate a report, which becomes quite complex and a large task.
It's no different to how it was before, and this task was to make it ansychronous.
Also most errors would be caused by a configuration error, it's already been validated by FDS, and these are reported to the admin.
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.
It is true. when i tried to track down the reason why jobs failed.. i had to iterate over all related items.. it would be not easy to find the "real" item which prevents the save
.
end | ||
end | ||
|
||
def fair_data_station_imports_to_show(project, contributor) |
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.
introduces an optional expiration_time
filter to limit the returned uploads to recent ones based on created_at
. This avoids cluttering the view with old failed or completed jobs, especially when retries are frequent.
.where('fair_data_station_uploads.created_at >= ?', expiration_time)
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.
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.
there is a close button X
to close them
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.
indeed.. didn't see it