Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automation for API routes generation #104

Open
MunifTanjim opened this issue Jun 6, 2018 · 19 comments
Open

Automation for API routes generation #104

MunifTanjim opened this issue Jun 6, 2018 · 19 comments

Comments

@MunifTanjim
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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 😃

@max-wittig
Copy link
Contributor

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
Copy link
Owner

jdalrymple commented Jun 7, 2018

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

@jdalrymple jdalrymple added this to the 4.0.0 milestone Jun 7, 2018
@jdalrymple
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link

aol-nnov commented Aug 6, 2018

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
Copy link

mdsb100 commented Aug 14, 2018

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
Copy link
Contributor Author

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

@mdsb100
Copy link

mdsb100 commented Aug 15, 2018

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

@jdalrymple
Copy link
Owner

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

@MunifTanjim
Copy link
Contributor Author

MunifTanjim commented Jun 6, 2019

@jdalrymple
Copy link
Owner

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

@MunifTanjim
Copy link
Contributor Author

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
Copy link
Owner

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

@MunifTanjim
Copy link
Contributor Author

MunifTanjim commented Jun 7, 2019

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
Copy link
Owner

True! Baby steps lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants