-
Notifications
You must be signed in to change notification settings - Fork 3
0.2 release #34
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
base: master
Are you sure you want to change the base?
0.2 release #34
Changes from all commits
590a5ee
41c9a14
18eca4a
2d99e27
9801d18
a922154
ed03656
cd59586
d9014b7
8497502
1442964
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| # TODO | ||
| remaining tasks for the `0.2` release, in order of priority | ||
|
|
||
| ## major | ||
| * [ ] move behaviours (getting, updating, etc.) to library methods | ||
| * [ ] validation (schema, column, relationship) | ||
| * [ ] declarative entity definitions | ||
| * [ ] schema validation | ||
| * [ ] documentation | ||
| * [ ] callbacks | ||
| * [ ] pagination | ||
| * [ ] hateoas-style support | ||
|
|
||
| ### move behaviours | ||
| at the moment, when you generate your code, a lot of functionality | ||
| gets put in the restplus endpoint. this results in huge diffs between | ||
| versions and is tiresome to test. | ||
|
|
||
| it is proposed that this code moves into a library function which then | ||
| gets imported by the endpoint. this will make the code easier to test, | ||
| and will result in a smaller generated codebase. | ||
|
|
||
| ### validation | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sounds lovely, in practice we've seen this being a huge problem in day to day use.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whoops, I massively typoed in there. In practice we've not seen this being a huge problem in day to day use. I think it would be nice, I'm not sure it's the highest priority for us. |
||
| there's loads that could be done to validate a schema. the most interesting | ||
| part is probably the relationships - to do this we would need a two-pass | ||
| implementation where the first pass builds up an idea of what entities are | ||
| present and how they relate to one another, and the second pass | ||
| verifies that these relationships are possible. | ||
|
|
||
| error reporting should be as approachable and helpful as possible. see | ||
| [elm](https://elm-lang.org/) for inspiration. | ||
|
|
||
| ### declarative entity definitions | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is hard to get right. There is a lot of additional information involved in putting together a a set of entities and decorator based interfaces are notoriously hard to get right, particularly if they're dealing with a lot of data.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. think this would be a much nicer interface than the current declarative approach. with the decorators I only mean for collecting the classes - the alternative to this is doing it in the SQLAlchemy style which I think is a lot less pleasant. would be fun to spike this out to see how it looks
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe I need to see it. It would be nice if it could be implemented initially as an additional interface that produces the current interface. That way it could introduced in a backwards compatible way. |
||
| writing entities declaratively is much more natural to python developers | ||
| already familiar with sqlalchemy, which is a dependency of genyrator. | ||
| it is proposed to use decorators in the style of | ||
| [attr.s](http://www.attrs.org/en/stable/) to configure the behaviour of | ||
| entities. | ||
|
|
||
| ### schema validation | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would strongly encourage against implementing yet another validation engine. There are two (three if you count jsonschema) as part of genyrator already (marshmallow and flask-resplus). I think it would be better if figured out how to lean on one of those more. Given that flask-restplus seems to be unmaintained now and marshmallow is widely used in the community my preference would be there. Even if it is a bit lacking.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agreed, would be nice if we could do this with an existing framework 👍
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. marshmallow-sqlalchemy has support for overriding fields and for manually generating fields (that function accepts keyword arguments that are passed on down to the marshmallow field) so this does not sound very difficult. |
||
| we currently validate provided data with a really old version | ||
| of marhsmallow-sqlalchemy. pros: it gives nice errors and will persist | ||
| the data if it's valid. cons: it's not easily configurable from the standpoint | ||
| of someone writing an entity. explore going with a different library, writing our | ||
| own or something different? there could be the opportunity to allow the option | ||
| to specify column-level validation - how would this look? | ||
|
|
||
|
|
||
| ### callbacks | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be interesting to step back and think of different ways of approaching this. Are callbacks the right answer or being able to fully override certain aspects. What are the actual user needs that we have come across? What would be most natural to a developer that already knows flask-restplus / flask / sqlalchemy / marshmallow ...?
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had this in here more as a place to stick ideas. the idea was that if you just want to customise a specific endpoint, or do a little more processing on all |
||
| provide a mechanism for intercepting the data at different points of processing. | ||
| perhaps as a first pass, just allow a user-provided method to be called at the start | ||
| of the endpoint method, and another before the response gets serialized? | ||
|
|
||
|
|
||
| ### documentation | ||
|
|
||
| it would be wonderful to have documentation which takes the time to explain | ||
| some of the core concepts of columns, entities and schemas, as well as | ||
| giving examples of how to configure different kinds of relationships. | ||
|
|
||
| ### pagination | ||
| configurable pagination on a per-endpoint basis. | ||
|
|
||
| ### hateoas-style support | ||
| location headers. explorable links. | ||
|
|
||
| ## minor | ||
|
|
||
| * [ ] stop using `.format`, start using `f` strings everywhere | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| from functools import lru_cache | ||
|
|
||
| from flask_sqlalchemy import SQLAlchemy | ||
|
|
||
|
|
||
| class DBNotInitialisedException(Exception): | ||
| message = "It looks like you're trying to use genyrator without having " | ||
| "initialised the db object first! Call `init_genyrator` with your SQLAlchemy " | ||
| "db instance before trying to use any of the generated code." | ||
|
|
||
|
|
||
| def init_genyrator( | ||
| sqlalchemy_instance: SQLAlchemy, | ||
| ): | ||
| get_db_instance._db_instance = sqlalchemy_instance | ||
|
|
||
|
|
||
| def get_db_instance(): | ||
| if not hasattr(get_db_instance, '_db_instance'): | ||
| raise DBNotInitialisedException(DBNotInitialisedException.message) | ||
| else: | ||
| return get_db_instance._db_instance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it. It would allow people to be much more granular about which bits of the Genyrator they use.