jdalrymple / gitbeaker

🦊🧪 A comprehensive and typed Gitlab SDK for Node.js, Browsers, Deno and CLI
Other
1.55k stars 294 forks source link

Automation for API routes generation #104

Open MunifTanjim opened 6 years ago

MunifTanjim commented 6 years ago

Have you thought about automating the whole process, like what octokit/rest.js is doing?

The upside is that, you no longer have to manually update API parameters and things like that. So, a lot of the problems like #53 #49 #101, won't occur anymore. You get the idea.

So, we'll use https://github.com/gitlabhq/gitlabhq/tree/master/doc/api these docs to generate machine readable json file like this: https://github.com/octokit/rest.js/blob/master/lib/routes.json

And use that in the library. It'd also be useful for creating API client with other languages.

If you like the idea, I can contribute.

(I'm doing something similar on https://github.com/MunifTanjim/node-bitbucket )

jdalrymple commented 6 years ago

I've thought about it in the past, as it would of saved alot of time and removed some possibility for missing support. The only reservation I had towards it was customization. Many of the endpoints abstract certain options for simplicity, so generically auto generating them from the docs would take some of this value away, at least IMO.

They do have some nice abstraction through the routes.json file which we could definitely use, but even though its clean, i don't think its really adds to the readability, though it would drastically cut down the package size.

For the most part, we've tried to account for api changes by having the generic options argument, but in the case of #101 for example, the option was forgotten.

Not sure what peoples opinions are on the difficulty of contributing to this project, whether is current state is straight forward or if something like this would make it easier/harder. Definitely something to think about.

MunifTanjim commented 6 years ago

The only reservation I had towards it was customization. Many of the endpoints abstract certain options for simplicity, so generically auto generating them from the docs would take some of this value away, at least IMO.

Well, abstraction can still be applied. If the routes definition is available, how you're gonna utilize it is totally up to you. I'd be able to explain further if I knew what type of abstractions you're talking about (because I've not read through all of the codes of this library yet).

They do have some nice abstraction through the routes.json file which we could definitely use, but even though its clean, i don't think its really adds to the readability, though it would drastically cut down the package size.

The routes.json is not supposed to be readable. As it's not written by hand. It's only there for the library to read. Also, the end-user (i.e. developers who use the library) experience is fantastic. I can do this with autocomplete:

octokit-rest-autocomplete

So, no need to go to the docs.

For the most part, we've tried to account for api changes by having the generic options argument.

The current design of generic options argument can be kept with generated routes.json too.

Not sure what peoples opinions are on the difficulty of contributing to this project, whether is current state is straight forward or if something like this would make it easier/harder. Definitely something to think about.

Currently there's so many files with repetition in codes. IMO, it'll be easier to maintain if we don't have to deal with hardcoding every api endpoint urls into different files.

MunifTanjim commented 6 years ago

The only reservation I had towards it was customization.

Also, I'd like as less customization as possible. Because, that removes the extra overhead of learning to use the api client for the end-user. So, if someone reads the official documentation of the api and then wants to use the api client library, the two should be matched as closely as possible.

But that's just my opinion. It's up to you how you'd like it to be :smiley:

max-wittig commented 6 years ago

python-gitlab also follows this priciple. This way new users of node-gitlab only need to understand the basics of the library and can read about the rest of the features in the GitLab API docs.

jdalrymple commented 6 years ago

Makes sense, perhaps this could be the next big thing to work on after the typescript and testing updates?

jdalrymple commented 6 years ago

Looked more into this over the weekend. Not sure how we could reliably pull the data from Gitlabs md files since they do alot of grouping which can be misleading. I really like the idea though, would cut down on alot of code as well. It it helps for portability and readability I'm 100% down for it.

The only thing I can forsee myself not liking is having all the routes in one file. Personally i find large files REALLY hard to digest. We could have multiple files following the same JSON format, each file representing a route and use a build step to combine the files for publishing. That way its easily digestible when developing for it, but can be more or less exported for any port in the future. Thoughts?

MunifTanjim commented 6 years ago

Not sure how we could reliably pull the data from Gitlabs md files since they do alot of grouping which can be misleading.

The gitlab-ce repository is huge. So the doc/api directory is extracted from it in this repo: gitlab-api-docs. It is automatically updated daily by a corn job. This repository is used for generating routes.

And here's a rough implementation of the routes generation from the gitlab-api-docs: gitlab-api-routes. It'll need further refinements before we can start using it.

The only thing I can forsee myself not liking is having all the routes in one file.

JSON file is generated for each markdown doc. So, it's already separated by sections.

Personally i find large files REALLY hard to digest. We could have multiple files following the same JSON format, each file representing a route and use a build step to combine the files for publishing.

If we generate a JSON file for each routes, then there will be hundreds of them! Maybe just separating them by sections is enough? (There are about 60 markdown files already).

Also, if the routes generation process spits a specific format structure (which it will), you won't need to read through every routes while developing, just reading a few of them will be enough as all of them will have the same format structure.

jdalrymple commented 6 years ago

If we generate a JSON file for each routes, then there will be hundreds of them! Maybe just separating them by sections is enough? (There are about 60 markdown files already).

Sorry for the confusion, I did mean for each section, not each route. lol that would be madness.

What you have in gitlab-api-routes is pretty much what I wanted to do. It would be wicked to have that generation of routes as a npm library, with cli execution, such that we could update the routes in the library easily with a command. I would step away from having it as part of a ci step just because every time the ci would run, the routes could possible change breaking a whole bunch of tests that depended on older or deprecated routes.

aol-nnov commented 6 years ago

For automatic api client generation please monitor this merge request.

I think swagger support would be a huge plus to the gitlab api, but unfortunately I'm not sure if it gonna happen any time soon.. :(

mdsb100 commented 6 years ago

There is a compromise solution: Auto create Users.js (class) by router and create API by decorators. Like Interface-Oriented Programming.

Decorators is readable.

Example:

@cls(someNameSpace, orUseFulInfomation)
class Users extends BaseService {
  @api({ options: true, action:"GET" })
  all(/*options*/) {
    //return RequestHelper.get(this, 'users', options);
  }

  @api('<userId>', { options: true, action:"GET" })
  events(/*userId, options*/) {
    //validateEventOptions(options.action, options.targetType);

    //const uId = encodeURIComponent(userId);

    //return RequestHelper.get(this, `users/${uId}/events`, options);
  }
}
MunifTanjim commented 6 years ago

N.B.: Decorators proposal is still at Stage 2 (Draft).

mdsb100 commented 6 years ago

This project used babel originally, so there is no problem by using decorator.

jdalrymple commented 5 years ago

Follow up, is there a way to have types for JSON objects properties?

MunifTanjim commented 5 years ago

Yes.

https://gitlab.com/MunifTanjim/gitlab-api-routes/blob/218087ff3b9d2b876d0d72095b303883f4072e06/routes/pipelines.json#L148-153

They can be used for validation before sending api requests.

jdalrymple commented 5 years ago

But typing wouldn't be useful for developers though...

MunifTanjim commented 5 years ago

But typing wouldn't be useful for developers though...

Why do you think that? It can be used to generate typescript definition files. And developers will be able to use it with intellisense. Like this:

jdalrymple commented 5 years ago

Oh I didnt know that! How can we generate definition files from that? Or is that something the typescript compiler does automatically?

MunifTanjim commented 5 years ago

With the help of this package: json-schema-to-typescript

The main challange is to generate the json schema correctly. Because GitLab doesn't provide OpenAPI Specification and their documentation is not uniformly structured (different pages has different formatting, so it'll have many corner cases while parsing). But I think it's doable.

These two repositories will help as a starting point:

jdalrymple commented 5 years ago

True! Baby steps lol