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

Update sdr client #3083

Merged
merged 3 commits into from
Jun 15, 2023
Merged

Update sdr client #3083

merged 3 commits into from
Jun 15, 2023

Conversation

peetucket
Copy link
Member

@peetucket peetucket commented Jun 9, 2023

Why was this change made? 🤔

Use latest sdr-client gem to handle invalid JSON files

Fixes #3075

[HOLD] until sul-dlss/sdr-client#284 is merged and a new release is cut, then come back here and update the gem file to use the correct version, re bundle install and update this PR.

Also requires sul-dlss/sdr-api#585

How was this change tested? 🤨

on QA

@peetucket peetucket changed the title Update sdr client [HOLD] Update sdr client Jun 9, 2023
@peetucket peetucket marked this pull request as draft June 9, 2023 19:34
# ActiveStorage is expecting "application/x-stata-dta" not "application/x-stata-dta;version=14"
content_type&.split(';')&.first
end

Copy link
Member Author

Choose a reason for hiding this comment

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

this logic is now handled in the new version of sdr-client, so we don't need it here anymore

Copy link
Member

@jmartin-sul jmartin-sul Jun 14, 2023

Choose a reason for hiding this comment

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

nice! ok, yeah, i like moving this into sdr-client, despite my wishy-washy feedback on the sdr-client PR. hadn't looked at this yet, so wasn't sure whether there'd be redundant processing (which i was more worried about for intelligibility than for performance reasons).

@peetucket peetucket force-pushed the update-sdr-client branch from f11629f to 41cd256 Compare June 15, 2023 16:11
@peetucket peetucket changed the title [HOLD] Update sdr client Update sdr client Jun 15, 2023
@peetucket peetucket marked this pull request as ready for review June 15, 2023 16:11
Copy link
Member

@jmartin-sul jmartin-sul left a comment

Choose a reason for hiding this comment

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

approving, but holding off on merge till sul-dlss/sdr-api#585 is merged

@jmartin-sul jmartin-sul merged commit 935e30e into main Jun 15, 2023
@jmartin-sul jmartin-sul deleted the update-sdr-client branch June 15, 2023 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

User can't deposit JSON files unless they contain valid JSON
3 participants