Make load_sample_data more resilient

[dhruvkb]

Fixes

Addresses #2359 by @sarayourfriend

Description

This PR makes these changes:

• ensures that invalid API calls to the ingestion server using cURL fail locally for non-2xx response codes.
• checks for sample data to be loaded in SQL after issuing DELETE and \copy to the database.

This PR does not rewrite the script in a different language, like Python, for two reasons:

• It has been tried before (Fix broken load script and port to Python openverse-api#830) but it was turned down as it was too much rewriting for the little value gained.
• The current structure of the script is largely a collection of just recipe invocations. Switching to Python will not improve any inherent fragility in those recipes.

Checklist

• My pull request has a descriptive title (not a vague title like Update index.md).
• My pull request targets the default branch of the repository (main) or a parent feature branch.
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added or updated tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no visible errors.
• I ran the DAG documentation generator (if applicable).

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

[zackkrida]

One major error resulting from a typo, but it's such a quick fix that I'll commit it and give this the approval.

Edit: typo fixed in b53e266

[sarayourfriend]

This is a nice change, but I don't think it resolves the linked issue. Calls to the ingestion server return a task ID. If that task fails the script will continue on without issue. The call to ingest the test data, for example, only checks if the index is created afterwards before moving on, not that the task actually finished or succeeded.

The reason to convert to Python is that it would be way easier to parse the request body for the task ID and make repeated requests to the task endpoint of the ingestion server. The PR you mentioned didn't do that either, to be fair, and I think overcomplicated things by reusing the just recipes and adding new functionality, rather than converting the script as is. I don't think that previous PR is necessarily a good comparison or an indication that re-writing to Python doesn't make certain aspects of it easier. The linked PR did a lot more than re-write to Python and add checks on the task IDs.

Anyway, this is a change that we should merge, but I don't think it fully resolves the issue as the tasks can still fail and the script would go on anyway.

[sarayourfriend]

I was surprised by this because I assumed cURL supported some kind of method for changing the exit code if the status was bad. It turns out there's a relatively new feature (2021) that does this: --fail-with-body. It has the added benefit of still printing the response body with the failure without needing to add any more flags (or checks).

[dhruvkb]

I didn't use that because a lot of us are developing on macOS and might have an older version of cURL built-in. Installing a new version of cURL is technically an option but I experienced SSL issues when I last did that and I didn't want to force an additional system-level dependency.

[sarayourfriend]

Ah, TIL that macOS is on an old version 👍

[dhruvkb]

Not only cURL, macOS comes bundled with plenty of old utils.

I also had problems with Git when I tried to use some new feature (can't recall what it was), didn't have it, installed Git with Brew which then refused to work with SSH proto. It was a whole thing to reset everything 😢.

[dhruvkb]

I changed "Fixes ..." to "Addresses ..." to leave the linked issue open after merging this PR.