search

tuomur / python-odata

A simple library for read/write access to OData services
MIT License
79 stars 59 forks source link

Fix issues in EntitiesList & Update URL #4

Closed awartani closed 7 years ago

awartani commented 7 years ago

Thanks for the great code, without this we were not able to connect to Microsoft Dynamics 2016 in Python.

I don't have a huge experience in oData standers but notices few issues when used it with Microsoft Dynamics Web APIs.

  1. EntitiesList is empty, even when reflect_entities=True. This might be related to the metadata file that's part of Dynamics CRM 2016(I can provide the file).

The logic of creating Entities is not clear, maybe you can add some comments about how did you thought about it.

  1. When perform update operation the url contained something like /Accounts('id') and the request was always rejected until the url was escaped from the single quotes to be /Accounts(id)
  2. When entities are being added using entity_name instead of collection_name, some Entities were overwritten and lost, this might be a reason of the point #1 but the lack of knowledge in oData standers makes it harder for me to understand it.
awartani commented 7 years ago

@tuomur I am willing to continue working on this, the solution attached might be not the best but if you can provide additional information and guidance I am willing to spend more time to write a proper fix.

tuomur commented 7 years ago

Not sure about the metadata changes, but removing quotes from instance URL is not correct. Entities can have strings as primary keys and strings must be quoted in query URLs. Northwind example: http://services.odata.org/V4/Northwind/Northwind.svc/Customers('ANTON')

Are you sure your service implements Odata 4.0? The older versions had a lot of differences that are not handled in this library.

awartani commented 7 years ago

Thanks @tuomur, Based on Microsoft Dynamics CRM 2016 it's based on oData v4.0

I attached the metadata file here so you would be able to take a look.

ODataV4Metadata.xml.zip

What do you think about the other issues? should the string be encoded somehow?

tuomur commented 7 years ago

Looking at your metadata example Dynamics seems to use aliases in metadata names, and the current metadata parser code does not support those. That's most likely the reason for reflection and other things not working.

I'm not really keen on adding implementation-specific things to the library, so maybe these could be implemented in a separate metadata class?

awartani commented 7 years ago

@tuomur a lot of issues were solved by only adding 'Edm.Guid': UUIDProperty to metadata class. Good to know about aliases, and yeah, I think it should be in it's own logic but integrated with metadata class.

awartani commented 7 years ago

@tuomur I can add comments to what I think is important and what should be ignored. BTW, this is a great library and I wish you can contribute more to it. It satisfies our needs but I am sure a lot more people would use specially for integrate with Dynamics 2016.