OpenAlchemy icon indicating copy to clipboard operation
OpenAlchemy copied to clipboard

support for the array type

Open anovis opened this issue 5 years ago • 4 comments

sqlalchemy supports the array type for postgress https://docs.sqlalchemy.org/en/13/core/type_basics.html#sqlalchemy.types.ARRAY

would be mapped from openapi array type https://swagger.io/docs/specification/data-models/data-types/#array

happy to submit a pr for this if you think its worth it

anovis avatar Mar 31 '20 18:03 anovis

Thanks for submitting the proposal, I do think it is a good idea. The only note is that this would touch a lot of areas in the code base. At a high level:

  • [ ] The array type currently indicates a relationship between two schemas, this assumption would need to be updated to perform a type check before assuming that is the case.
  • [ ] The from_dict and to_dict utility functions similarly expect relationships for array property types. This would need to be put behind a type check.
  • [ ] The default values currently only support basic types, this would need to be updated and would result in requiring arrays to be mapped to appropriate Python types.
  • [ ] Producing the typed models file also assumes that arrays are relationships. This would need to be updated to support arrays for simple types.

There are also still some open questions around which sub types are supported within the array.

I'm happy for you to work on a pr, just be mindful that this is going to require familiarity with the code base and would require quite a number of test cases to be written. It would also require some thinking around integration tests as they are currently run against SQLite. The options are (1) introducing a test dependency for Postgres, (2) using some sort of docker image or (3) reserving this for the Azure based integration tests. I'm leaning towards (3) with a potential for (2) but I'm not sure if the added complexity is worth it.

jdkandersson avatar Apr 01 '20 10:04 jdkandersson

thanks for the reply and all the details! super helpful. I think i will hold off for now on the pr then and instead spend some time to get more familiar with the code base, especially on the tests.

anovis avatar Apr 04 '20 02:04 anovis

Hi @anovis. I was wondering if you had any updates on the status of this PR. I am interested in using this feature once it is available. Thanks!

astepe avatar Dec 28 '20 18:12 astepe

@astepe I ended up not having enough getting time to make much progress. I may try to give it a go in the future though

anovis avatar Feb 18 '21 18:02 anovis