Conversation
|
Great ideas. I like the idea of pulling the logic out of the resources. Would the intention be to handle the API paths in a similar way? |
|
yeah, we could definitely handle the api paths similarly, there's not too much logic in there right now so it should be easy to pull it out into a separate method I think? |
the future * moved "db_import_path" string to "db_import_statement" * added spec for crerating schema with a DBImport * this is a backwards-compatability breaking interface change 😱
* this means the changes are no longer bc-breaking * now you have to call `init_genyrator` before you can start using the app
db injection
|
|
||
| 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. |
There was a problem hiding this comment.
Love it. It would allow people to be much more granular about which bits of the Genyrator they use.
| gets imported by the endpoint. this will make the code easier to test, | ||
| and will result in a smaller generated codebase. | ||
|
|
||
| ### validation |
There was a problem hiding this comment.
This sounds lovely, in practice we've seen this being a huge problem in day to day use.
There was a problem hiding this comment.
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.
| error reporting should be as approachable and helpful as possible. see | ||
| [elm](https://elm-lang.org/) for inspiration. | ||
|
|
||
| ### declarative entity definitions |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
| [attr.s](http://www.attrs.org/en/stable/) to configure the behaviour of | ||
| entities. | ||
|
|
||
| ### schema validation |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
agreed, would be nice if we could do this with an existing framework 👍
There was a problem hiding this comment.
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.
| to specify column-level validation - how would this look? | ||
|
|
||
|
|
||
| ### callbacks |
There was a problem hiding this comment.
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 ...?
There was a problem hiding this comment.
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 GETs or something like that, you could sneak your own code in.
|
The ideas in the README sound good. By the amount of additional code that seems to be going on in this PR is the intention to implement everything in here and then do one massive merge? I would be very nervous about that. What is plan for this branch? |
my plan was to implement most of it, would be good to split out the file to show what is the minimum and focus on those things but I think validation (of payload and schema) and moving out the behaviours would be a really good first start. and then do one massive merge :)
tbh my plan was to try to maintain backwards-compatibility but to break things where necessary. without access to a large existing implementation, I will have to rely on tests to see if it's going to break things. |
|
I was thinking about how other projects manage change and wondered whether we could manage these features with issues and address each separately with PRs. That way we have an easy to navigate history of larger change items. It also allows us to make a feature by feature call about whether they can be implemented in a backwards compatible way (I think they all can). In terms of how to manage those that can't. Could we, at the point that we are going to introduce a backwards compatibility breaking change cut a |
3 participants
a holding PR for everything included in the
0.2release