noirbizarre / flask-restplus

Fully featured framework for fast, easy and documented API development with Flask
http://flask-restplus.readthedocs.org
Other
2.73k stars 507 forks source link

Flask-restplus future organization and roadmap #612

Open noirbizarre opened 5 years ago

noirbizarre commented 5 years ago

Hello everyone :wave:

Some have been wondering if this project is dead (#593 #418). No it's not but maintaining an open-source requires some time (at least). As a consequence, I added some maintainers (@ziirish @SteadBytes @a-luna @j5awry ) and we need to start figuring out how we can make the situation evolve. I can add more maintainers if others have some time to spend.

This issue will serve as a temporary discussion space on future organization and roadmap.

I will complete this messages with taken decisions and directions until everything enters the documentation.

a-luna commented 5 years ago

An obvious roadmap item (as called out in the current documentation) is the removal of the reqparse module. The docs state that new documentation explaining how to integrate other serialization/validation packages with restplus will be created. I'd be happy to work on this.

YJinHai commented 5 years ago

340

I think that needs to be on the agenda, thank you

ziirish commented 5 years ago

I think we need to work on #223 since this is a recurring question/problem. A first step is to add some examples in the documentation. Then we can think whether this should be handled by Flask-restplus or not.

SteadBytes commented 5 years ago

I have created #615 after investigating #570 . It is quite misleading to have empty namespaces in the Swagger UI so i think it should be addressed :+1:

j5awry commented 5 years ago

I'm spending time today going through issues more. I'm also going to spend more time with the code today, and see what's up.

definitely want to second the removal of the reqparse module, and movement to other marshalling things. This also ends up causing issues with documentation, as sometimes when you're marshalling with json schema, the docs don't fully load as expected (though i'm going through if we're doing something wrong there).

j5awry commented 5 years ago

Ok, some stuff I'm seeing that seem pretty important:

556 (more so the ability to set JSON schema)

438 and #538 and anything else that has marshalling libraries.

462 and other general GET model type issues (we're seeing one with JSON Schema not being shown in GET endpoints)

414 (and using JSON schema in general)

SteadBytes commented 5 years ago

@j5awry, anything to do with marshalling and reqparse is definitely something we should prioritise as any changes to that functionality may also have a large impact on other roadmap items (Swagger documentation, validation etc) :+1:

j5awry commented 5 years ago

Everyone, So i mentioned I work for a large company. That company is Akamai Technologies. As part of my non-compete clause, any work I do in the open source community is technically owned by Akamai Technologies. I'm in the process of getting final approvals, and sorting out what it is I need to do to contribute. So far, I believe this is the general statement:

  1. update the LICENSE file to include a line

Additional work, Copyright (c) Akamai Technologies, 2019

  1. When I make commits, add in the message the copyright notice. that way, if something changes in the license, the Akamai work can be removed. I don't see this becoming a big issue.

This means the current maintainers have to agree to add the extra copyright and be ok with the corporate sponsorship. It'll also open things up for more folks from Akamai to contribute (there are at l least 5 of us on our team using this regularly, so there's a pool of folks there :) ). And sometimes a little corporate sponsorship can be a good thing.

SteadBytes commented 5 years ago

@j5awry, that's really great to hear that you're pushing for approval ๐Ÿ‘ To clarify, would that change to licence mean that Akamai has the right to remove any code that you were to contribute?

j5awry commented 5 years ago

Yes. It's pretty standard for companies with non-competes, to my knowledge. I'm trying to do things "right." If I can't do them "right" it means my time for contributions is pretty close to nil.

Anyway, Akamai has never removed any code from open source projects. The only time it becomes an issue is if the license of the software moves from something open source (like the BSD 3 restplus has now) to something proprietary. At that point, legal has to get involved. But as long as everything stays open sources under an acceptable license (BSD, MIT, Apache, etc) then there shouldn't ever be any issues in the future. It's really when things go from open to closed source it'd all go awry. I can toss some examples of Akamai's contributions in here if folks would like (they include terraform, linux kernel, openssl etc).

SteadBytes commented 5 years ago

I understand and appreciate that you are trying to do things right - I think that is definitely the right way to approach this :smile: I apologise if it sounded like I was dismissing your attempts to contribute, that wasn't my intention. I was just trying to ensure that I had the correct understanding of the implications of the licence changes :+1:

I personally don't have an issue with this, though I think the final decision is for @noirbizarre to make.

On another note, since we are merging some outstanding PR's for bug fixes what do people think of relasing some of the new changes in a minor version bump?

ziirish commented 5 years ago

Hi,

I have no problem with the license changes either though I think this is a decision for @noirbizarre

I also think this might be a good signal for the community if we released a new bugfix version. But one of my PR's was merged back in October and this was not a minor change.

iamzjk commented 5 years ago

Iโ€™m using restplus at work, and did some customizations. Would love to contribute once we get a roadmap

SteadBytes commented 5 years ago

Agreed @ziirish , a release would be a very good signal to the community, especially if it contained long needed bugfixes :smile: @noirbizarre what do you think?

j5awry commented 5 years ago

I'll open a PR for the License change, and we can go from there.

Would folks would be amenable to starting a slack, discord, or other chat service rather than going nonstop on this ticket? It might help us with faster communication.

Also 100% agree on the release. We should go through commits, and make a choice where our release should be.

ziirish commented 5 years ago

You can use Gitter. At lease @SteadBytes, @noirbizarre and I are already sitting there.

ziirish commented 5 years ago

What do you think about #250?

I personally like it :+1:

ziirish commented 5 years ago

I have started to put what has already been selected in this thread here. I haven't assigned any priority to those issues yet.

I will open a PR in order to introduce ourselves as new collaborators to the project. Then we should add a checkpoint tag to release a new "let's reborn" version. That will allow us to start implementing the roadmap.

We still need you @noirbizarre for the release process though, because only you can publish on pypi.

ziirish commented 5 years ago

Here is the PR: #622

SteadBytes commented 5 years ago

Looks great, thanks @ziirish - are there any bugs in particular that you have seen that we should address immediately?

I'm hoping to get a PR for #615 done for the bug fix release ๐Ÿ‘

g0di commented 5 years ago

Hello guys, I'm happy to see some changes about flask-restplus. I saw you talked about some migration for serialization/deserialization/validation, in particular, integrating other frameworks.

I already successfully forked flask-restplus and integrated it with marshmallow (serialization), apispec (swagger spec generation) and webargs (request parsing). I tried to tie to flask-restplus "way" but it may not fit exactly what you want. Anyway, feel free to have a look at it, it may help you. Please note that I removed all the original code relative to model/request parsing. Moreover, I did not refactored all the swagger generation code to fit apispec "way" but it could be a good solution to simplify the whole.

https://github.com/EarthLab-Luxembourg/flask-restplus/tree/marhshmallow-webargs-apispec-integration

Formartha commented 5 years ago

I'm using flask-restplus and I'd like to see some actual integration with a frontend service like Django :)

flayman commented 5 years ago

Hi all. My company has been developing micro-services using flask-restplus for a couple years now and I've become involved in that effort more recently. I like what Marshmallow provides for serialization to and from SQLAlchemy and so I've been interested in that aspect. I checked out the flask-restplus-marshallow project, but it doesn't seem to be very mature. I took the apispec approach to converting Marshmallow to JSON Schema and that in turn to SchemaModel, but I was disappointed to find that SchemaModel based classes are not supported for response marshalling. There are also a number of issues with JSON schema in Swagger, some of which I tried to address in a PR. Also the conversion to JSON Schema is a bit simplistic.

In terms of integration with Marshmallow, @hiboo's project looks very interesting. I think however that a more palatable approach from the upstream point of view would be to preserve the existing Model hierarchy and to allow marshmallow.Schema derived objects to be placed on an equal footing with SchemaModel and ModelBase, pretending for a moment that SchemaModel actually is on an equal footing. From @hiboo's perspective ditching the original model and request parsing code would have greatly simplified his task. I would be interested in contributing in any way that I can towards a solution that ticks all of the boxes. I don't foresee any ownership issues.

SteadBytes commented 5 years ago

@flayman Thank you for using (and contributing to) Flask-RESTPlus! Migrating to using Marshmallow is very much top of the roadmap for version 2. I agree that preserving the existing Model hierarchy is a good approach, however from an implementation perspective I don't think it has been given much thought as of yet. I'm hoping to get some time this week to take a closer look at the problem and prototype some possible implementations :+1:

Our lead maintainer @noirbizarre is currently on paternity leave and upon his return we will be doing real planning of the roadmap and features for version 2 :smile:

Thanks again for you comments, if you have any more thoughts or want more in depth discussion about anything Flask-RESTPlus (i.e. due to your usage at work) please don't hesitate to jump on Gitter!

flayman commented 5 years ago

Hi Ben (@SteadBytes). Let me say congratulations to @noirbizarre on his latest development project! Sleep is overrated. ;-)

Today I've been looking at how to possibly allow SchemaModel to provide marshalling capability. I have something sort of working by expanding the class to have fields like the other types. I can convert a json hierarchy into a combination of fields.Nested and fields.Raw fairly easily. Tomorrow I was going to see about taking that further and using the more specific field types. However, json schema is much more complex and flexible than the Model structure can accommodate. I don't know whether it's worth the effort to take it much further. Unfortunately there does not seem to be a good solution currently in python for coercion to json schema. I'm talking about json schema instead of marshmallow, but you can get json schema from marshmallow, so it's a decent compromise.

RemiDesgrange commented 5 years ago

Do you consider moving the repo under it's own org ?

Also. Right now, we are in 0.12 version. Maybe setting the version to 1, and moving to 2 when marshmallow/webargs/whateverelse migration will be complete. I can open another issue to discuss this.

SteadBytes commented 5 years ago

Hey @flayman , I know we've discussed a bit on Fitter but I do apologise for not replying to your comment here. I would love to take a look at what you've got so far if you like? From what I've looked into it does look like marshmallow -> JSON Schema might be the best way to go.

flayman commented 5 years ago

Hi @SteadBytes. I'm not sure that Marshmallow to JSON Schema will give good results all around. It's a way in, but the Marshmallow plugin leaves a bit out of the conversion. I think there are two separate concerns here. Both would be good to support. On the one hand is Marshmallow, which is great for serialization to and from a data store and thus should be a choice for request handling and response marshalling. On the other hand, JSON Schema is very expressive and all the model schemas wind up as JSON schema anyway. Marshmallow is unlikely to be able to exploit all of the features of JSON Schema and it's probably also difficult for flask-restplus to use JSON Schema natively for marshalling, but swagger-ui seems to understand it up to quite a recent version of the draft. Someone has ripped out all of the model and request parsing code of flask-restplus and replaced it with Marshmallow, so clearly that can work. It should however sit beside the other model hierarchy rather than replace it. That's what I think, anyway. Others may disagree.

flayman commented 5 years ago

Rather than edit the distributed flask-restplus or pull my own from GitHub into my pip requirements, I decided to just monkey-patch and replace the SchemaModel class to be able to do marshalling. I'd be happy to provide that for you to look at. It's far from perfect but it's currently satisfying my own requirements. I also have a test suite for it which is some way short of complete.

SteadBytes commented 5 years ago

@flayman Hmm yes I see your point, the Marshmallow -> JSON Schema is certainly not perfect. Yes, using Marshmallow for marshalling requests/responses will definitely work. The difficulty (which we've already experienced with current Models) is also providing this functionality when defining a model using JSON Schema. Am I correct in my understanding that you're suggesting that both model hierarchies are supported but separate? For example, one can define a model using JSON Schema or Marshmallow. Both will provide valid JSON Schema in the resulting swagger specs and both should be used for request/response validation. I guess the JSON Schema models may not be appropriate for marshalling in terms of converting/formatting data to different structures like the current Models can do ๐Ÿค”

flayman commented 5 years ago

Yes, I was suggesting you treat Marshmallow integration separately to the JSON Schema route, which is partially implemented. JSON Schema requires a bit more functionality to be on par with the built in models. Marshalling is missing and although the docs suggest you can use SchemaModel for that, if you try to use it you end up with an error because the model is not iterable. It also needs the refs problem sorted out. Marshmallow is not integrated at all, but if it were it would be able to do everything that was required. Someone has done that but not in a backwards compatible way.

Back to JSON Schema, the marshalling problem is not trivial. How do you marshall to a schema that uses oneOf or anyOf? It's not so hard to validate, but it requires a bit of thought to coerce a data set. There is a npm JavaScript project called Ajv which claims to do coercion to a schema as well as validation. We could take a look at how they do it. I tried looking into how swagger-ui produces examples as it seems related, but I quickly got lost in the woods.

flayman commented 5 years ago

Here's a gist with my partial solution for marshalling with SchemaModel. Needs to be imported before any of the flask_restplus modules. It changes the inheritance hierarchy of SchemaModel and makes it iterable with fields.

https://gist.github.com/flayman/e92d88e63e45a0d8b6cd1c41c48dc6c0

JaDogg commented 5 years ago

IMHO Can we move this to a new organisation (FlaskRestPlus or something similar) so it's easier for new maintainers to continue working on it, add any other relevant projects (Some plugins maybe?) to same organisation. Archive the this repo and point to repo in FlaskRestPlus organisation. (That repo can be a fork of this)

This is a very good project. Good luck. ๐Ÿ‘

j5awry commented 5 years ago

We do have a gitter, please join!

https://gitter.im/noirbizarre/flask-restplus

The repo itself is doing well with contributions. We do need more people, and should probably schedule some actual meetings to figure things out. Our current blocker is PyPi keys, and our release workflow. We need that in place so, as an org, we can release the changes we've pulled in.

I've been on vacation a bit, and will be back more fully in reviewing and, hopefully, contributing code.

SteadBytes commented 5 years ago

I have been away a bit recently too, however should be back to normal levels of contribution soon! ๐Ÿ˜€

We've got a lot of changes merged into master however as @j5awry said the issue at the moment is actually making a release on PyPi. @noirbizarre has the only credentials for that and is still taking a break for paternity leave. I have reached out a couple of times but (as I'm sure he's very busy!) been unable to get an ETA on his return or permissions for us other maintainers to make PyPi releases ๐Ÿค”

@j5awry, arranging some actual meetings would definitely be a good idea ๐Ÿ‘ - it would be very helpful if we could have @noirbizarre there too

smarlowucf commented 5 years ago

Hi :wave: , It's nice to see things are getting on track! If there's a need for more maintainers I can help out. For now I will provide some pull requests.

As far as the issue with PyPi releases. Could versions in the mean time be cut and tagged in GitHub then pushed up to PyPi once the permissions are resolved?

a-luna commented 5 years ago

Hi all! I was added as a maintainer when this thread began but havenโ€™t had time until now to get involved. Please let me know if thereโ€™s anything I can do to start helping out and please let me know about any planning meetings so I can start contributing!

SteadBytes commented 5 years ago

@smarlowucf Yes I think making some releases on GitHub will be a good idea, at least then users can install directly from a GitHub release if need be.

I think the current set of features/bug fixes that have been merged in should make up a release. We can then plan out upcoming work more carefully.

@j5awry what do you think?

RemiDesgrange commented 5 years ago

@SteadBytes making a tag and telling users how to install it via pip ?

j5awry commented 5 years ago

On the list of documentation we need is how to cut a release. We definitely have enough pulled in to necessitate a release. It'd be nice to use Github's release feature, and maybe even do more complex pipeline stuff that'd automatically distribute to Pypi when certain conditions are met (new features in Github launched for CI/CD just this week!)

Short term, yes, let's get a tag cut

tincumagic commented 5 years ago

I have been away a bit recently too, however should be back to normal levels of contribution soon!

We've got a lot of changes merged into master however as @j5awry said the issue at the moment is actually making a release on PyPi. @noirbizarre has the only credentials for that and is still taking a break for paternity leave. I have reached out a couple of times but (as I'm sure he's very busy!) been unable to get an ETA on his return or permissions for us other maintainers to make PyPi releases

@j5awry, arranging some actual meetings would definitely be a good idea - it would be very helpful if we could have @noirbizarre there too

I don't think that @NoirBizare is on paternity leave because he is working ... it has contribution activity the whole time for other projects if you check his profile: August 8, 2019 opendatateam/udata 5 commits etalab/datagouv-search-indicator 2 commits Optimize CSV generation

We have to contact him to help with a new pip release.

SteadBytes commented 5 years ago

@RemiDesgrange Yes exactly, pip can install directly from a GitHub tag:

pip install -e git://github.com/noirbizarre/flask-restplus.git@{ tag name }#egg={flask-restplus}

@j5awry I agree 100% on all of that, automating releases to PyPi is definitely the goal here - we just need access to it :thinking: Shall we arrange that meeting to talk through the state of things in a bit more detail?

@tincumagic Yes it seems so, that may just mean that he's still taking a break from Open Source projects outside of his day job. Although some communication would be appreciated. I reached out to him a couple of weeks ago and got no reply. I have tried again via Twitter, though if there is no response I'm not sure what we can really do except for starting a new fork :thinking:

ziirish commented 5 years ago

Also, we can now use API tokens to upload on Pypi as documented here

What do you think @noirbizarre?

SteadBytes commented 5 years ago

I have reach out to @noirbizarre via twitter and have received a response - he has made a new release to PyPi :tada: and has popped up on Gitter.

a-luna commented 5 years ago

Nice! Version 0.13.0 is available on PyPI

apryor6 commented 5 years ago

I've written a library that allows you to use Marshmallow within RESTplus called flask_accepts. I was directed towards this thread from the Gitter channel, and I'm sharing the link here in case any of the code or concepts are found useful.

Without going into the weeds, flask_accepts is built on top of RESTplus and adds functionality for converting Marshmallow schemas into RESTplus Models, documenting them accordingly in Swagger.

flayman commented 5 years ago

Hi @apryor6. This looks great. I've tried to incorporate it into my project and I've noticed a couple things that don't work. First, it requires marshallow>=2.19.5, which was fine before Marshallow released major version 3. Pip now installs the latest version of Marshallow, which breaks compatibility with client modules like marshmallow-sqlalchemy. It may also be incompatible with your module. Second and last, your decorators do not work with Marshmallow schema instances. It will be important to allow that, since you can pass keyword parameters to a schema instance constructor such as "only" and "exclude".

Using a scheme instance with accepts results in:

  File "/group13/mattf/git-projects/adfs/venv/lib/python3.6/site-packages/flask_accepts/utils.py", line 32, in for_swagger
    for k, v in vars(schema()).get("declared_fields", {}).items()
TypeError: 'MySchema' object is not callable
Using a schema instance with responds results in:
  File "/group13/mattf/git-projects/adfs/venv/lib/python3.6/site-packages/flask_accepts/utils.py", line 54, in get_default_model_name
    return "".join(schema.__name__.rsplit("Schema", 1))
AttributeError: 'PersonSchema' object has no attribute '__name__'

These should be very easy to fix, and I will turn this comment into a request on your project.

apryor6 commented 5 years ago

Thanks, @flayman -- for the sake of thread continuity see issues here and here

Edit: Both issues have been addressed, and it now supports Marshmallow 3+

flayman commented 5 years ago

That was quick!

alvarolopez commented 5 years ago

I am not a maintainer, therefore this is just my opinion: As a suggestion, in order to foster contributions, I would rather remove the need of adding the contributed changes to the CHANGELOG.rst file. Basically this causes a lot of troubles when merging pull requests, since every single time there is a conflict and people need to rebase the changes. Using git it should be quite straightforward to generate a changelog with the relevant changes whenever a release is made.