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

Flask-restplus future organization and roadmap #612

Open
noirbizarre opened this issue Mar 26, 2019 · 51 comments
Open

Flask-restplus future organization and roadmap #612

noirbizarre opened this issue Mar 26, 2019 · 51 comments

Comments

@noirbizarre
Copy link
Owner

noirbizarre commented Mar 26, 2019

Hello everyone 👋

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

a-luna commented Mar 27, 2019

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

YJinHai commented Mar 27, 2019

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

@ziirish
Copy link
Collaborator

ziirish commented Mar 27, 2019

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

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 👍

@j5awry
Copy link
Collaborator

j5awry commented Mar 28, 2019

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

j5awry commented Mar 29, 2019

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

SteadBytes commented Mar 29, 2019

@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) 👍

@j5awry
Copy link
Collaborator

j5awry commented Apr 3, 2019

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

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

j5awry commented Apr 3, 2019

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

I understand and appreciate that you are trying to do things right - I think that is definitely the right way to approach this 😄 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 👍

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

ziirish commented Apr 5, 2019

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

iamzjk commented Apr 6, 2019

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

@SteadBytes
Copy link
Collaborator

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

@j5awry
Copy link
Collaborator

j5awry commented Apr 9, 2019

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

ziirish commented Apr 9, 2019

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

@ziirish
Copy link
Collaborator

ziirish commented Apr 10, 2019

What do you think about #250?

I personally like it 👍

@ziirish
Copy link
Collaborator

ziirish commented Apr 10, 2019

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

ziirish commented Apr 10, 2019

Here is the PR: #622

@SteadBytes
Copy link
Collaborator

SteadBytes commented Apr 10, 2019

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

g0di commented Apr 16, 2019

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

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

@flayman
Copy link

flayman commented Jun 11, 2019

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

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

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

flayman commented Jul 9, 2019

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

flayman commented Jul 9, 2019

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

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

flayman commented Jul 12, 2019

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

flayman commented Jul 12, 2019

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

JaDogg commented Aug 6, 2019

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

j5awry commented Aug 6, 2019

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

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

Hi 👋 , 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
Copy link
Collaborator

a-luna commented Aug 8, 2019

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

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

RemiDesgrange commented Aug 9, 2019

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

@j5awry
Copy link
Collaborator

j5awry commented Aug 9, 2019

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

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

SteadBytes commented Aug 11, 2019

@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 🤔 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 🤔

@ziirish
Copy link
Collaborator

ziirish commented Aug 11, 2019

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

What do you think @noirbizarre?

@SteadBytes
Copy link
Collaborator

SteadBytes commented Aug 13, 2019

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

@a-luna
Copy link
Collaborator

a-luna commented Aug 13, 2019

Nice! Version 0.13.0 is available on PyPI

@apryor6
Copy link

apryor6 commented Aug 22, 2019

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

flayman commented Aug 23, 2019

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

apryor6 commented Aug 23, 2019

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

flayman commented Aug 23, 2019

That was quick!

@alvarolopez
Copy link

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.

@JaDogg
Copy link

JaDogg commented Oct 16, 2019

Would it be possible to follow a good commit guideline so CHANGELOG can be generated?

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