New install/refresh API with the store

pedronis · September 26, 2017, 10:31pm

ATM installing a snap uses the /details store API, while refreshes use the /metadata API. Since we have grown some more requirements that made us consider reviewing these APIs:

we need to send validation context (all currently installed snaps) for all operations, so we avoid an install choosing a revision ignoring validation constraints but the later refresh immediately switching to a different validated revision
we are also switching to sticky ignore-validation flags for snap to ask the store not to enforce validation
the special case of snap refresh --revision is using the /details API but given epochs we would like to send the current revision (and so implicitly epoch) as well

The proposal for a new API is to have a single bulk endpoint for both install and refreshes. A request in JSON to it would look like this:

{
"context": [  // information about the current installed snaps
{
     "snap-id": 
     "revision":  // the current revision
     "tracking-channel":
     "ignore-validation":  // current flag about enforcing validation for the snap
},...],
"actions": [  // call this "intents" instead ?
{
    "action": "install"|"refresh|refresh-all"  // distinguish auto-refresh vs cmdline "snap refresh" ?
    // if action is install
    "name":
    "channel":  // optional, defaults to stable
    "tag":   // ? tag where from is the install requested
    "ignore-validation":   true|false  // defaults to false
    //  if action is refresh
    "snap-id":
    // common
    "revision":  // explicit requested revision (optional)
    // if action refresh-all
}...],
"fields":  [...]  // which detail fields to send in the response? do we need install-fields vs refresh-fields?
}

The response would look like:

{
"results": [{
  "snap-id":
  "name":
  "result": "install"|"refresh"|"error"
  "message":  // if result is error
  "snap": {
     ... // requested detail fields
  }
}...],
}

These request/response assume we would start switching for new store APIs to use “-” for field names instead of “_”.

Some open questions:

final naming of request/response fields
do we need to be able to specify different field sets for the response entries about installs vs refreshes?
do we want to distinguish a snap refresh vs auto-refreshes in the actions?
really switch to use “-” instead of “_”

niemeyer · October 6, 2017, 7:05pm

Thanks for the notes. Adding to upcoming.

facundobatista · October 24, 2017, 11:39am

Hola Samuele! Thanks for starting this thread.

We started to work on this, have two questions and some behaviour details (mostly to see if we’re in the same page):

Questions:

Why you need to send the epoch to te server? IMO this doesn’t make sense anymore as the snap is not “in an epoch” anymore, now epochs is just two sets of capabilities (read & write), and the server already has this info, so…
What is the “tag”? How is it useful or used server side?

Details:

I’m still collecting the list of fields that could be returned for the snap revision, but the “channel map” is not needed at all in this response, right?
If there is an error for any snap (e.g. you are not allowed to access it) the response would be 200 in general, but only that snap will be in error state, while the rest of the response is useful, right?
The server will return a 400 in the following cases:
- a “refresh-all” action was combined with a “refresh” and/or “install” actions
- the same snap is included in the “install” and “refresh” list
- a snap is included in the “refresh” list, but it’s not included in the Context
(for the last two items we could just error on the specific snap, but we’re proposing to completely fail as if you’re mixing the snaps that way you may have a deeper problem when building the request)

What do you think? Thanks!

pedronis · October 24, 2017, 11:50am

about sending epoch, I think it will be the two fields explode likely, I think we discussed this at the rally and @niemeyer asked it for completeness, also not directly related but we might have to deal with refreshing out of a local install, in which case revision wouldn’t be useful or missing in context and the server would need to consider the explicit epoch info.

not super important for now, is just information about the “source” of the install, like a marketing campaign tag from an install url but we discussed that

correct

sounds reasonable

facundobatista · October 24, 2017, 11:58am

You say that “revision” is not mandatory in the context?

pedronis · October 24, 2017, 12:01pm

I don’t know, just saying we have that corner case, it might be that it is an install but the install has an epoch field which is to be used as a constraint. My main point is that we have cases where we have an epoch constraints that can come only from the client.

ricardokirkner · October 24, 2017, 12:03pm

Do you think epoch should be required then? What if the epoch values conflict with what we have server-side for the given revision?

facundobatista · October 26, 2017, 1:04pm

So we need to use the request’s “epochs” if it comes from the client, as it may be different from what we have in the server for that revision.

We could make it optional, and if not present get it from the DB, but you think this complexity worths it? Or just make it mandatory?

Also, now we’re talking about epochs… what would be it’s structure?

Option A, one complex field: "epochs": {"read": [...], "write": [...]}
Option B, two simple fields: "epochs_read": [...], "epochs_write": [...]
Option C, ??

pedronis · October 26, 2017, 1:07pm

no, I discussed with Gustavo yesterday, and we are thinking to leave out epochs from context, there will still be some special case where the client will pass epoch info but it will be in the operation object and we don’t need to worry about it in the first pass.

facundobatista · October 26, 2017, 1:42pm

Awesome, thanks!

(this text is only to reach the 20 chars minimum imposed by the forum infrastructure)

pedronis · October 26, 2017, 2:35pm

I removed “epoch” from the context in the request (in the top post).

facundobatista · October 26, 2017, 2:42pm

This is the list of available fields that could be selected using the fields field in the request:

architectures
base
binary_filesize
binary_path
binary_sha3_384
binary_sha512
confinement
created_at
created_by
epoch
is_released
revision
snap_id
snap_yaml
type
version
was_released

Two notes regarding field:

if field is not included, the response will include ALL snap revisions fields (this is very useful in the default case, or when exploring by hand, as it’s an easy way to check which are the available fields or their spelling)
if field is included with an empty list, no specific fields would be returned: each item in the response list will have snap id, name , result (and maybe message) but no snap entry

pedronis · October 26, 2017, 3:15pm

I’m quite sure there’s missing stuff there that we can get with details today, I need to check what snapd really needs.

facundobatista · October 26, 2017, 3:38pm

Please, if you could verify this ASAP it would be great, because it impacts heavily in which internal services and databases we consume to retrieve the needed data. Thanks!

pedronis · October 26, 2017, 3:39pm

I can try, but @chipaca is off, what exactly is needed depends also how we plan to get metadata (summary, description etc), atm we are getting them with what are now the install/refresh API calls

pedronis · October 26, 2017, 3:48pm

atm snapd is also getting at least:

name (already covered)
channel (I imagine it might be different from the requested one when a channel is closed)
contact
title
summary
description
private (bool)

also for downloads:

all the information about deltas:

type snapDeltaDetail struct {
      FromRevision    int    `json:"from_revision"`
      ToRevision      int    `json:"to_revision"`
      Format          string `json:"format"`
      AnonDownloadURL string `json:"anon_download_url,omitempty"`
      DownloadURL     string `json:"download_url,omitempty"`
      Size            int64  `json:"binary_filesize,omitempty"`
      Sha3_384        string `json:"download_sha3_384,omitempty"`
}

and global download url and anon download url (don’t know if they are different anymore with snaps though)

facundobatista · October 30, 2017, 5:08pm

Conclusion of a meeting we had today between @ricardokirkner , @chipaca , @pedronis and me…

We started with the basis of these fields that are the ones currently being returned from the Store for the metadata endpoint:

anon_download_url
architecture
binary_filesize
channel
channel_maps_list
confinement
content
deltas
description
developer_id
download_sha3_384
download_url
epoch
icon_url
last_updated
origin
package_name
prices
private
publisher
ratings_average
revision
screenshot_urls
snap_id
summary
support_url
title
version

Decisions:

Will add a snap_yaml field
Will remove these ones: publisher, channel_maps_list, origin, prices, and ratings_averages
Will transform epoch field, from a string to: {'read': [...], 'write': [...]}
Will change some names:
- content to type
- package_name to name
- support_url to contact
- in general: change _ to - in composed names

We need some further analysis on the following…

We have download_url, why there is also an anon_download_url? Is there a difference? In which cases?
Currently there is a developer_id field… What about publisher_id? Do we need both? One of those? Which one?

ricardokirkner · November 1, 2017, 6:45pm

In order to simplify the API a bit more and stop cargo-culting implementation details from past solutions we will now:

return ‘app’ instead of ‘application’ as the ‘type’ field (‘application’ was mostly returned for CPI compatibility, which is no longer needed in the new api)
return ‘null’ for base if that field wasn’t specified in the revision’s yaml file (this would otherwise be a single exception in how nullable fields are treated)
return ‘snap-name’ instead of ‘name’ (it was considered a plain ‘name’ could be confused with the snap’s title)

pedronis · November 1, 2017, 7:19pm

it’s a bit strange though that we take “name” in the input and return “snap-name” in the output, there’s a precedent though in snap-declaration to use “snap-name” instead of just “name”, otherwise the only thing commonly prefixed with “snap-” is “snap-id” (we rarely tend to use/say just “id”)

ricardokirkner · November 1, 2017, 7:26pm

I agree… I was going to ask if we could not instead use ‘snap-name’ for the input as well. This would also make the api more self-consistent (snap-id and snap-name instead of snap-id and name).