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

support for the array type #116

Open
anovis opened this issue Mar 31, 2020 · 4 comments
Open

support for the array type #116

anovis opened this issue Mar 31, 2020 · 4 comments
Labels
enhancement New feature or request

Comments

@anovis
Copy link

anovis commented Mar 31, 2020

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

@jdkandersson
Copy link
Owner

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 jdkandersson added the enhancement New feature or request label Apr 1, 2020
@anovis
Copy link
Author

anovis commented Apr 4, 2020

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.

@astepe
Copy link

astepe commented Dec 28, 2020

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!

@anovis
Copy link
Author

anovis commented Feb 18, 2021

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants