Json package report try #2

[imalsogreg]

Replacement for #399

Add two endpoints, which reuse existing URLs but use the requested content type to choose a JSON response instead of HTML. The new endpoints provide basic information about a package (author, description, license), and a listing of available versions for a package, along with each version's deprecation status.

For example:

/package/avro-0.4.1.2

{
    "author": "Thomas M. DuBuisson", 
    "description": "Avro serialization and deserialization support for Haskell", 
    "license": "BSD-3-Clause", 
    "metadata_revision": 0
}

/package/avro

{
    "0.4.1.1": "normal", 
    "0.4.1.2": "deprecated", 
    "0.4.1.4": "normal"
}

For the first (single-version) view, the URL can specify a metadata revision:
/packages/avro-0.4.1.1/revision/1

{
    "author": "Thomas M. DuBuisson", 
    "description": "Avro serialization and deserialization support!", 
    "license": "BSD-3-Clause", 
    "metadata_revision": 1
}

(when no revision is specified, the most recent one is used)

NOTE: There is no caching implemented for these endpoints. Is it Ok to assume that the computations being done (parsing several cabal files per request) are cheap enough to justify not adding some AcidState caching?

There is no dependencies data in this PR. I'm thinking of adding that in a second PR, after talking with @hvr about his ideas for formatting there.

/cc @hvr @alexcmcdaniel @gbaz

[imalsogreg]

There's at least one error: package/avro returns the JSON result even when we aren't requesting json.

[alexcmcdaniel]

Would it be possible to include homepage as well?

[imalsogreg]

Fixed and cleaned up. Ready for review :)

[hvr]

there is no caching implemented for these endpoints. Is it Ok to assume that the computations being done (parsing several cabal files per request) are cheap enough to justify not adding some AcidState caching?

Depends... there's some packages which have 150+ releases and there's also .cabal files which are not that cheap to parse and take a significant amount of time and space to parse. But I don't think you need to parse all .cabal files for any single service request here?

Btw, when there's a description field, I'd also expect the synopsis-field to be present. And if you extract the license-information, then you also ought to extract the copyright: property imo.

[imalsogreg]

@hvr re: synopsis and copyright, agreed! Done.

re: caching, yes, requests for the version listing only force parsing of the cabal files for that package. Requests for a particular version only parse that one version. Would it be too optimistic to assume that when we are only using these top-level fields, laziness saves us from having to parse the whole file?

[alexcmcdaniel]

I noticed that the sample endpoints do not have .json, is that intentional?

[imalsogreg]

@alexcmcdaniel Thanks, yep. If you just type those example URLs into a browser you'll get html. Implicitly I meant for an Accept: application/json header to be attached to the requests.

[alexbiehl]

Would it be too optimistic to assume that when we are only using these top-level fields, laziness saves us from having to parse the whole file?

Yes. To claim a successful parse the whole file needs to be examined. Usually lazy parsing only works well if your format supports laziness explicitly.

[alexcmcdaniel]

@imalsogreg any updates?

[imalsogreg]

@alexcmcdaniel I'm slowly working on the caching part.

[alexcmcdaniel]

@imalsogreg how is the caching coming?

[imalsogreg]

I added AcidState actions for the API endpoints. It does read-through caching, with a hook on package change to delete the cache lines for a package.

I did not add any backup logic, since all the data that would be backed up is a redundant view of other already-backed-up data. And our API is cheap to call. So the backup would cost us more in maintenance and versioning than it would help us with site integrity. Think so?

I've rebased to clean up the commit history, added a couple small tests and gone through a cleanup pass. Is there anything else we should do before final review and merge? Any more fields to add to the package description API?

[alexbiehl]

Can we make these fields strict? We put that into a map in memory, it would be a shame to introduce a leak.

[alexbiehl]

Also it might make sense to use Data.Map.Strict here.

[imalsogreg]

Note: You should use Data.Map.Strict instead of this module if:

You will eventually need all the values stored.
The stored values don't represent large virtual data structures to be lazily computed.

Sounds like me! 👍

[imalsogreg]

Anyone available to approve/request changes? I'd love to get this in to hackage :)

[alexbiehl]

@iamalsogreg can you explain why we wouldn't use Map.delete here?

[imalsogreg]

Yah - since descriptions is keyed on (PackageIdentifier, Maybe Int), if I use delete, I would be deleting just a particular package/metadata combination. But what I want is for a change to the package to delete every package entry for some fixed package, at every metadata revision.

If there were a deleteWithKey function that transforms the key before deciding what to delete, that could be nicer than the filterWithKey I use.

[alexbiehl]

Ah makes sense, I didn't see both fst!

[alexcmcdaniel]

@imalsogreg hey sorry to make a last second request but would it be possible to add the homepage as well?

[alexcmcdaniel]

oh never mind I see that you added it in the last commit, any update on the timing of the release? @imalsogreg @hvr

[aj-arena]

I am also interested in this pull request, any update?

@gbaz

[gbaz]

obsoleted by #996