Thread: Add versions.json endpoint with latest release information

Hi Sehrope,

On 4/27/21 2:25 PM, Sehrope Sarkuni wrote:
> Hello,
>
> The attached patch adds a /versions.json endpoint with the latest
> release information. The response is a machine parseable JSON array with
> one entry per release.
>
> For simplicity the patch keeps the same field names in the response as
> the django model. Someone else with more familiarity with django may
> want to change that (tree => major?).

Thanks for proposing this. This seems reasonable. I wanted to see if we
could have leveraged "VersionFeed" for this, but TIL the django
syndication views[1].

I would suggest modeling the query after what we are doing with the RSS
feed[2].

Thanks,

Jonathan


[1] https://docs.djangoproject.com/en/3.2/ref/contrib/syndication/
[2]
https://git.postgresql.org/gitweb/?p=pgweb.git;a=blob;f=pgweb/core/feeds.py;hb=HEAD

On Wed, Apr 28, 2021 at 3:29 PM Sehrope Sarkuni <sehrope@jackdb.com> wrote:
>
> On Tue, Apr 27, 2021 at 5:09 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:
>>
>> I would suggest modeling the query after what we are doing with the RSS
>> feed[2].
>
>
> Done. Updated patch is attached.
>
> It also renames some output fields ("tree" as "major" and "latestminor" as "minor") and adds a sort order on the JSON
fieldsto ensure stable output.
 
>
> $ curl -s http://localhost:8000/versions.json | jq last
> {
>   "current": false,
>   "eoldate": "2014-07-01",
>   "major": "8.4",
>   "minor": 0,
>   "reldate": "2009-07-01",
>   "supported": true
> }

I like the idea, btu a few comments:

* Why do we care about sorting the keys? Not that it matters much, but
it mostly seems a bit silly, but if there is a reasoning behind it
there's no problem -- I'd just like to know what the reasoning is.

* It reports major version as "13.0", not "13". It's correct for
versions prior to 10.

* In the XML/RSS feed we include a link to the release notes for the
version. Do we want to do that here as well? I'm thinking probably not
because (1) the RSS needs a clickable link, that's probably the only
reason it's there, and (2) release notes can now be found at
/docs/release/<version>/, which was not the case at the time of the
RSS feed being created. But I wanted to raise it for discussion in
case  people would find it useful?

* I'm thinking the orignal name "latestminor" is actually better than
"minor" -- the previous minor releases still exist after all.

* Would it make sense to have an actual query parameter like
"?current=1" or something that would give you *just* the current
version? To avoid having to download all? And maybe also
"?supported=1" to get just supported versions? To avoid having to do
those filterings  client side?

-- 
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/

On 4/30/21 2:58 AM, Magnus Hagander wrote:
> On Wed, Apr 28, 2021 at 3:29 PM Sehrope Sarkuni <sehrope@jackdb.com> wrote:
>>
>> On Tue, Apr 27, 2021 at 5:09 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:
>>>
>>> I would suggest modeling the query after what we are doing with the RSS
>>> feed[2].
>>
>>
>> Done. Updated patch is attached.
>>
>> It also renames some output fields ("tree" as "major" and "latestminor" as "minor") and adds a sort order on the
JSONfields to ensure stable output.
 
>>
>> $ curl -s http://localhost:8000/versions.json | jq last
>> {
>>    "current": false,
>>    "eoldate": "2014-07-01",
>>    "major": "8.4",
>>    "minor": 0,
>>    "reldate": "2009-07-01",
>>    "supported": true
>> }
> 
> I like the idea, btu a few comments:
> 
> * Why do we care about sorting the keys? Not that it matters much, but
> it mostly seems a bit silly, but if there is a reasoning behind it
> there's no problem -- I'd just like to know what the reasoning is.
> 
> * It reports major version as "13.0", not "13". It's correct for
> versions prior to 10.

Then this needs to be corrected:

https://www.postgresql.org/support/versioning/

"Minor releases are numbered by increasing the last part of the version 
number. Beginning with PostgreSQL 10, this is the second part of the 
version number, e.g. 10.0 to 10.1; for older versions this is the third 
part of the version number, e.g. 9.5.3 to 9.5.4."

> 
> * In the XML/RSS feed we include a link to the release notes for the
> version. Do we want to do that here as well? I'm thinking probably not
> because (1) the RSS needs a clickable link, that's probably the only
> reason it's there, and (2) release notes can now be found at
> /docs/release/<version>/, which was not the case at the time of the
> RSS feed being created. But I wanted to raise it for discussion in
> case  people would find it useful?
> 
> * I'm thinking the orignal name "latestminor" is actually better than
> "minor" -- the previous minor releases still exist after all.
> 
> * Would it make sense to have an actual query parameter like
> "?current=1" or something that would give you *just* the current
> version? To avoid having to download all? And maybe also
> "?supported=1" to get just supported versions? To avoid having to do
> those filterings  client side?
> 


-- 
Adrian Klaver
adrian.klaver@aklaver.com

On Fri, Apr 30, 2021 at 1:06 PM Sehrope Sarkuni <sehrope@jackdb.com> wrote:
>
> On Fri, Apr 30, 2021 at 5:59 AM Magnus Hagander <magnus@hagander.net> wrote:
>>
>> * Why do we care about sorting the keys? Not that it matters much, but
>> it mostly seems a bit silly, but if there is a reasoning behind it
>> there's no problem -- I'd just like to know what the reasoning is.
>
>
> It's sorted to ensure a completely stable HTTP response. It shouldn't matter or be relied upon by any external
consumerof the data, but it's mildly convenient to know that the handler response will be byte-for-byte consistent
regardlessof Python version or if the response is cached. It makes testing a tad easier too as you can rely on an "=="
onthe raw response text. 

Hmm. Testability definitely is a good point.


>> * It reports major version as "13.0", not "13". It's correct for
>> versions prior to 10.
>
>
> Is there an updated set of sample data? I cloned the project and followed the dev setup instructions, but the initial
datait loads only goes up to 8.4. 

Yeah, the fixtures are in a sad state.

We should really try to get a process to handle that, as it's now
extremely ad-hoc.  I've added a script for it and refreshed the
current ones now so if you pull you'll at least get what's right at
this moment, even though it'll rapidly get outdated again. But please
try it and see if it breaks :)


>> * I'm thinking the orignal name "latestminor" is actually better than
>> "minor" -- the previous minor releases still exist after all.
>
> I was back and forth on this as well. I figured the endpoint itself represents the latest for each major version so
havingit be "minor" made more sense. 

Hehe. I had the same pattern of thinking, but came to the other conclusion :)

Jonathan, want to cast a deciding vote?


> A separate endpoint with all historical minor versions could then also have the same structure.

It could, but we don't actually keep track of historical minor
versions anywhere at this point. It might be interesting to do so, but
that's a bigger and separate patch, so let's not get that in the way
of getting this one approved.


>> * Would it make sense to have an actual query parameter like
>> "?current=1" or something that would give you *just* the current
>> version? To avoid having to download all? And maybe also
>> "?supported=1" to get just supported versions? To avoid having to do
>> those filterings  client side?
>
>
> My own use case would use the full list though I can see both "current" and "supported" being useful as well.
>
> If we structure the URL to support that in the future I think that's fine. Rather than query parameters could have
totallyseparate end points: 
>
> /versions/all.json - Array response with latest of each major version
> /versions/supported.json - Array response with list of supported versions
> /versions/current.json - Object response of only current version
>
> I don't know which of those would play nicer with how the HTTP caching works. The expectation is that these endpoints
aregoing to be hit repeatedly by things like CI platforms so we should get that part right. 

It should make no practical difference wrt caching. It's still just 3
URLs -- it would get bad if we supposed a huge mix of combinations,
but we wouldn't.

That said, thinking more about it maybe that's all over engineering
things. Anybody who's going to want a json file back is going to be
able to do the parse/filter step easily. So the only real reason to
separate them would be to reduce bandwidth -- and I can't believe that
a gzipped (transfer encoding) json file with all our versions is going
to be big enough to cause any issues..

--
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/

On Fri, Apr 30, 2021 at 3:11 PM Adrian Klaver <adrian.klaver@aklaver.com> wrote:
>
> On 4/30/21 2:58 AM, Magnus Hagander wrote:
> > On Wed, Apr 28, 2021 at 3:29 PM Sehrope Sarkuni <sehrope@jackdb.com> wrote:
> >>
> >> On Tue, Apr 27, 2021 at 5:09 PM Jonathan S. Katz <jkatz@postgresql.org> wrote:
> >>>
> >>> I would suggest modeling the query after what we are doing with the RSS
> >>> feed[2].
> >>
> >>
> >> Done. Updated patch is attached.
> >>
> >> It also renames some output fields ("tree" as "major" and "latestminor" as "minor") and adds a sort order on the
JSONfields to ensure stable output.
 
> >>
> >> $ curl -s http://localhost:8000/versions.json | jq last
> >> {
> >>    "current": false,
> >>    "eoldate": "2014-07-01",
> >>    "major": "8.4",
> >>    "minor": 0,
> >>    "reldate": "2009-07-01",
> >>    "supported": true
> >> }
> >
> > I like the idea, btu a few comments:
> >
> > * Why do we care about sorting the keys? Not that it matters much, but
> > it mostly seems a bit silly, but if there is a reasoning behind it
> > there's no problem -- I'd just like to know what the reasoning is.
> >
> > * It reports major version as "13.0", not "13". It's correct for
> > versions prior to 10.
>
> Then this needs to be corrected:
>
> https://www.postgresql.org/support/versioning/
>
> "Minor releases are numbered by increasing the last part of the version
> number. Beginning with PostgreSQL 10, this is the second part of the
> version number, e.g. 10.0 to 10.1; for older versions this is the third
> part of the version number, e.g. 9.5.3 to 9.5.4."

I don't understand what you mean. That one is correct, isn't it? For
13.3 for example, "13" is the major version and "13.3" is the minor
version. There is no "13.0" involved. And that's exactly what this
text says?

-- 
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/

On 5/4/21 6:41 AM, Magnus Hagander wrote:
> On Fri, Apr 30, 2021 at 1:06 PM Sehrope Sarkuni <sehrope@jackdb.com> wrote:
>>
>> On Fri, Apr 30, 2021 at 5:59 AM Magnus Hagander <magnus@hagander.net> wrote:

>>> * I'm thinking the orignal name "latestminor" is actually better than
>>> "minor" -- the previous minor releases still exist after all.
>>
>> I was back and forth on this as well. I figured the endpoint itself represents the latest for each major version so
havingit be "minor" made more sense. 
>
> Hehe. I had the same pattern of thinking, but came to the other conclusion :)
>
> Jonathan, want to cast a deciding vote?

Split the difference and do "latestMinor" so it at least follows JSON
convention?

>> A separate endpoint with all historical minor versions could then also
have the same structure.
>
> It could, but we don't actually keep track of historical minor
> versions anywhere at this point. It might be interesting to do so, but
> that's a bigger and separate patch, so let's not get that in the way
> of getting this one approved.

Agreed with Magnus.

>>> * Would it make sense to have an actual query parameter like
>>> "?current=1" or something that would give you *just* the current
>>> version? To avoid having to download all? And maybe also
>>> "?supported=1" to get just supported versions? To avoid having to do
>>> those filterings  client side?
>>
>>
>> My own use case would use the full list though I can see both "current" and "supported" being useful as well.
>>
>> If we structure the URL to support that in the future I think that's fine. Rather than query parameters could have
totallyseparate end points: 
>>
>> /versions/all.json - Array response with latest of each major version
>> /versions/supported.json - Array response with list of supported versions
>> /versions/current.json - Object response of only current version
>>
>> I don't know which of those would play nicer with how the HTTP caching
works. The expectation is that these endpoints are going to be hit repeatedly by things like CI platforms so we should
getthat part right> 
> It should make no practical difference wrt caching. It's still just 3
> URLs -- it would get bad if we supposed a huge mix of combinations,
> but we wouldn't.

I would side with "keep it simple" for the time being, and move forward
with what was originally proposed by this patch (with feedback addressed).

> That said, thinking more about it maybe that's all over engineering
> things. Anybody who's going to want a json file back is going to be
> able to do the parse/filter step easily. So the only real reason to
> separate them would be to reduce bandwidth -- and I can't believe that
> a gzipped (transfer encoding) json file with all our versions is going
> to be big enough to cause any issues..

+1 on the overengineering aspect. It'd be fairly easy to monitor how
much of a bandwidth suck this is (and likely it's little).

Jonathan

On Tue, May 4, 2021 at 11:49 PM Sehrope Sarkuni <sehrope@jackdb.com> wrote:
>
> Updated patch is attached with the renamed field.
>
> Rather than use annotate(...), it defines a separate function to convert the Version model into a dict for
serialization.That way the numtree function can be used for the major version (i.e. for 10.0 => 10). I think it ends up
beingcleaner anyway as all the fields in the resulting JSON are listed in one place. 

Thanks, that looks much better, I agree.


> Thanks for updating the sample data. New output is now:

Applied, with a few small changes:

* Didn't pass the pep8 tests we run -- needed some adjustments to
indentations and spacing only
* Made version_to_json return the dict directly instead of building it
incrementally. Probably faster, but the important part being I find
that much more readable.
* renamed eoldate -> eolDate and reldate -> relDate to follow the
pattern Jonathan suggested wrt the latestMinor field

Thanks!

--
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/