fortran-lang / registry

Registry for Fortran package manager
MIT License
8 stars 3 forks source link

Some api specifications #15

Closed minhqdao closed 1 year ago

minhqdao commented 1 year ago

@arteevraina @henilp105 @perazz

I've built and deployed a simple backend so I could implement the parsing on the fpm side. As that is basically done, there are a few things to consider in terms of the api so everything works flawlessly:

json = {
  "cached_versions": ["0.1.0", "0.2.0"]
}

curl <url> -d json -o <output>
response = {
  ...
  "code": 200,
  "version": "0.1.0",
  "tar": "https://...",
  ...
}
arteevraina commented 1 year ago

@minhqdao Thanks for sharing this. I already have this API ready. I will refactor it and merge it inside a single API and ask you to review it.

arteevraina commented 1 year ago

@minhqdao Can you on the url /base_url/packages/namespace/package_name be able to send "GET" request as well ?

So, for the API when you basically don't have anything in the list of cached_versions (which means the list can be empty as you mentioned) you just send the GET request and get the latest version in response and in case you already have something in the cached_versions list, you can make a POST request and the same API will handle that and send you the desired response.

minhqdao commented 1 year ago

https://stackoverflow.com/questions/28947132/is-it-okay-to-use-same-resource-name-for-both-get-and-post-rest-api

It seems like you can. 🙂

I'll change it in fpm now so it behaves the way you said.

arteevraina commented 1 year ago

https://stackoverflow.com/questions/28947132/is-it-okay-to-use-same-resource-name-for-both-get-and-post-rest-api

Yes, it's exactly what I am doing.

So, the thing is this API will also be used in the frontend React App to make a get request and it should only show the data of the latest version + version history as well. So, I thought if you can make the method type as method: "get" in fpm if we don't have cached versions in fpm and just use the same endpoint ?

But yeah, it's not set in stone. I can always adjust the API if you will have to change too many things in the fpm. Let me know about that as well.

@minhqdao

minhqdao commented 1 year ago

I already implemented it. If I cannot find cached versions, I make a GET request, if I can find cached versions, I make a POST request on the same endpoint, sending a list of cached_versions.

arteevraina commented 1 year ago

Here is the PR : https://github.com/henilp105/registry/pull/18.

Please feel free to review & request any changes.

@henilp105 @minhqdao

minhqdao commented 1 year ago

@arteevraina Is it deployed somewhere so I can try it out "in real world"?

arteevraina commented 1 year ago

@arteevraina Is it deployed somewhere so I can try it out "in real world"?

Sure. I will try to deploy it somewhere and share the url with you.

arteevraina commented 1 year ago

Hi @minhqdao , You can use this endpoint https://registry-apis-arteevraina.vercel.app/packages/fortranlang/postman . Here, fortranlang is the namespace and postman is the package name.

minhqdao commented 1 year ago

Thanks, I do get a response there although it's quite different from what I expected. Please document the final response scheme and I will make the necessary changes to parse it correctly in fpm.

I guess that POST https://registry-apis-arteevraina.vercel.app/packages/fortranlang/postman isn't finished yet, right? It doesn't deliver a status code and also the response scheme is very different from the GET to the same endpoint. Since we expect package data in both cases, I would expect the same response scheme.

tarball should always contain the final link of the tarball. So you could copy and paste to download it. That would be great.

I guess that GET https://registry-apis-arteevraina.vercel.app/packages/fortranlang/postman/0.0.X with X = {1, 2, 3} is not supposed to work yet, right?

arteevraina commented 1 year ago

Thanks for the review @minhqdao .

I have made the scheme for both GET and POST request same. So, it should be like this now.

{
    "code":  200,
    "data": {
            ...
           "latest_version_data": {
                "dependencies": [
                "string",
                " vegetables"
            ],
            "isDeprecated": false,
            "tarball": "postman-0.0.3.tar.gz",
            "version": "0.0.3"
             }
      }
}

There is some more metadata (like description and other stuff) that I am sending for now in the API but that is not required by fpm client. Right ?

I think as far the tarball is considered we have still not decided which storage service to use. But, I think now is the good to think about it. After that is decided I will replace the current string with link in the metadata so it should be fine after that.

GET API for specific version was supposed to work but I think there was some crash in the backend. I have fixed it and it should be working fine now. Schema for it looks something like this :


{
    "code: 200,
    "data": {
      ...
      "version_data": {
            "dependencies": [
                "string",
                " vegetables"
            ],
            "isDeprecated": false,
            "tarball": "postman-0.0.1.tar.gz",
            "version": "0.0.1"
        }
     }
}
arteevraina commented 1 year ago

I also just realized the url that I shared for testing APIs is just the preview url. Here is the final url that is in production you can test - https://registry-apis.vercel.app/

arteevraina commented 1 year ago

@minhqdao @perazz How is fpm planning to give the project to the backend ? I think flask allows single and multiple files uploads. But, the server will not be able to access the user's file system and do some manipulation there to generate .tar.gz file ? Can we generate the distribution file for example .tar.gz file and send the single file to the backend and it will take care of that ?

Is this something we can do on fpm side ? I think we can make use of tar command line for that. https://stackoverflow.com/questions/50338201/how-to-compress-and-tar-a-folder-in-linux

perazz commented 1 year ago

I would follow the example of other package managers and do most of the packaging and checking work on the fpm side, then create a compressed tarball that contains everything to be uploaded:

minhqdao commented 1 year ago

Thanks @arteevraina, the POST now works and I receive a similar scheme as the GET. I've also seen that I get a slightly different result for requesting versions (instead of latest_version_data, I get version_data). But that's ok, I think this is something I can work with if it stays like that (please make sure to document all of it). I'll implement it in fpm once we have the links for the tarballs and then we can check it there.

Considering the upload, of course we create and compress the tarball on the fpm side if we choose to use fpm for the upload. We could also do the checking in fpm. However, I think we need to check the package on the backend, too, because even if we use an API token such as cargo does, it'll just be an http request where we send data containing the token and the tarball. You could literally send any tarball using the same token via a curl request and completely bypass fpm for the upload. Or am I seeing things wrong? 🤔

So if that is the case, there is no point in verifying a package in fpm as it needs to be done on the backend again. We'd just pack the necessary components together and send it to the backend as a tarball. To verify a package on the backend, you untar it, unzip it and check it. 🙂 If it passes the check, the tarball is put in the database, otherwise send an error back with what was exactly wrong about the package.

I also think that the dry-run option in cargo is great. 😄

arteevraina commented 1 year ago

Considering the upload, of course we create and compress the tarball on the fpm side if we choose to use fpm for the upload. We could also do the checking in fpm. However, I think we need to check the package on the backend, too, because even if we use an API token such as cargo does, it'll just be an http request where we send data containing the token and the tarball. You could literally send any tarball using the same token via a curl request and completely bypass fpm for the upload. Or am I seeing things wrong? 🤔

Yes, exactly we would do the checking on the backend side. That I think is doable, the only thing I was worried about generating tarballs on the backend side. But, that fpm will handle now. I think everything is on the track now.

minhqdao commented 1 year ago

I'll mention it here again for documentation purposes. I think we can drop the POST now because you're sending me package data that I can use on the fpm side to decide whether to download the tarball or not after checking with the cache. Sending a list of cached versions has become unnecessary.

We will need a POST request for resolving real constraints in the future. But it'll look different than what we currently have.

minhqdao commented 1 year ago

I've checked the GET requests with the latest base url (https://fpm-registry.onrender.com) and it works. The integration for loading packages seems successful and I think we can close this issue.

Thanks, @arteevraina.

However, cold starts are very slow (probably 5-10 seconds) and the performance generally also doesn't seem spectacular. I think we should do sth about that in the long term.

arteevraina commented 1 year ago

I've checked the GET requests with the latest base url (https://fpm-registry.onrender.com) and it works. The integration for loading packages seems successful and I think we can close this issue.

Thanks, @arteevraina.

However, cold starts are very slow (probably 5-10 seconds) and the performance generally also doesn't seem spectacular. I think we should do sth about that in the long term.

Thanks for the feedback @minhqdao . Yes, as you said on render deployments are generally for our testing right now. It will not be set in stone for future release deployments and we will surely migrate to some other fast server hoisting service.

minhqdao commented 1 year ago

Let's share future updates to performance issues here: https://github.com/fortran-lang/registry/issues/18