Sounds good - I'll wait to hear back! I can't make any promises about deployment even after you and I have tested it, but this does seem like a nice thing to have, and I don't anticipate objections. Some timing factors are that we do have other code changes pending, along with a backlog of non-code work for LV admins that may come due at any time.

If you have questions on setting up the environment, there's a Contributing doc in this repo, and then there's much more (and I believe more up-to-date) info in the librivox-ansible repo.

If you'd like to address some cleanup in the meantime, I know Artom will appreciate if this PR comes as a single commit, making for a clean project history. One way forward would be to ask your favorite search engine or bot about using git squash, if you're not familiar... or you could just copy/paste the new lines into a new branch, whatever works for you. 😉

Hi,
Thanks for the details! I've installed the catalog locally and tested it with several books (including those without covers). It seems to be working properly – the coverart_jpg/coverart_pdf/coverart_thumbnail return the IA URL (or null if no cover is available). I've also squashed the changes into a single commit.
Thanks!

Just posting so you'll know I've seen this, and I'll likely test it in the next week or so. At the moment, it seems some of our Archive downloads aren't working, when it seems they should be, and we're also watching very closely for the time when we can start uploading the latest completed books. Isn't this exciting? 🤣

Thanks for taking the time to post this!

Perhaps I'm being overly cautious, but I'm always weary of making changes to APIs (both in general, and LibriVox specifically), even if, like this one, the changes are purely additive. I don't know how our APIs are being parsed and used, and foisting new fields on unsuspecting users isn't great.

I don't necessarily want to start versioning our API - that's too much complexity of the minimal amount of changes that we historically got - but perhaps we could at least make this change opt-in? I'm thinking something like a new cover_art parameter, that, if present and set to something True-like, would make us return the cover art info? That way, only users who know about this change and have opted in will receive the new API, while previous uses of the API will remain unchanged and 100% compatible.

What do you think? I'm open to ideas.

Thanks for the feedback! I've added a coverart parameter (?coverart=1). Please let me know if there's anything else that needs to be changed.

I was in the middle of typing, as the above change came in!

The message I had started on:

Hmm... I really don't know what the best way to go about it is. I see that we currently have a two-part selection process, when it comes to which info to return:

• Add: Client sets a flag to add fields - currently just one flag, 'extended'. It adds both project-level info (translators and genres), and section-level info (everything about sections is behind this flag).
• Filter: Client optionally a list of fields, and anything that is not included in the list (if provided) will not be returned by the API.

Here are a few ways we could move forward. Everything's a trade-off, and at least a little messy, depending how much we add.

1. Add as many new flags as we need (one per new field). Keep things otherwise the same.
2. Add one or more flags that follow logical groupings - "project level info" vs "section level info", for example, or perhaps as specific as "cover art" (edit: as above!). That's still a little less verbose than flag-per-field. The current 'expanded' flag stays as-is, whether it fits a theme or not.
3. Use the 'fields' list not just to filter, as it does now, but also to add to the selection. For clients that don't specify fields, or that specify existing fields, nothing changes. Clients that start providing the 'fields' list, so that they can specify new fields, will need to specify the old fields too, or they'll be filtered out.
4. Instead of adding nuances to the existing 'fields' list, add a new one that does the adding ('additional-fields'?). It's almost like adding a flag per field! A field that is present in this list would be excluded from the existing filter list. It's not a v2, but it's close-ish.

That's all in terms of ideas from me. Artom, if you have a preference and it isn't # 2, I'd be willing to help make it so, rather than ask fakerybakery to do that much for the 3 fields they want.

And, it appears the check has failed due to a temporary network error (can't download Wordpress). I can't clear it to run again, and I also won't be available to test this version until at least next week, but at least it's seen. 😅

And, it appears the check has failed due to a temporary network error (can't download Wordpress). I can't clear it to run again, and I also won't be available to test this version until at least next week, but at least it's seen. 😅

Not quite - WordPress.com took over ACF and renamed it, changing its URL and making the one we were using to download it invalid. There was a whole kerfuffle about it, even made its way onto Hacker News [1]. LibriVox/librivox-ansible#33 fixes it.

[1] https://news.ycombinator.com/item?id=41821336

@redrun45 as far as I know, the standard practice way of evolving APIs is to version them. In my OpenStack day job, we use a thing called "microversions". I believe the same overall idea applies elsewhere, even in web apps. The principle is that for every version of the API, the set of things that can be requested, as well as the possible responses, are well defined and fixed. Any chance to the API, be it in input or output, is a new version.

If we get to the point that we have enough changes to our API, we can move in that direction. I think that for now, what @fakerybakery has proposed is good enough.

[1] https://docs.openstack.org/api-guide/compute/microversions.html

@fakerybakery I'm fine with this as-is. Before merging, would you be willing to add tests for your new API? We have an old code base with a lot of gaps to cover, but at least for new things we're trying to move towards having tests where reasonable, and this is the perfect candidate for a test :) If not, either @redrun45 or myself can follow up with a separate PR.

Fair enough. I suppose "versioning the API" is exactly what all of those options would be, where I thought I was reducing complexity vs something like "/api/<major.minor>/". I've watched a few projects, but apparently from too much of a distance to know why they would do that. 🤷

In any case, I'd also be willing to add this to the '/api/info' doc, if that's not on the table yet.