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

Using pyodata with old MS CRM 2015 #95

Open
bip91 opened this issue Apr 19, 2020 · 16 comments
Open

Using pyodata with old MS CRM 2015 #95

bip91 opened this issue Apr 19, 2020 · 16 comments
Labels
bug Something isn't working

Comments

@bip91
Copy link

bip91 commented Apr 19, 2020

Hi,

Thank you for this wonderfull piece of code.
pyodata can also be used successfully with MS CRM 2015 with 2 minors changes :

  • the first one is to change PATCH method by MERGE Method in service.py (MS CRM 365 is now using PATCH)
  • the second one is a small try/catch in EdmStructTypeSerializer.from_json to avoid some None result
    that raise an exception (At the end of this report)

Is there a way to integrate these changes in a vendor specific python file as it exists for SAP ?

Change made in def from_json(edm_type, value):

            if type_prop.name in value:
>               try:
                   result[type_prop.name] = type_prop.typ.traits.from_json(value[type_prop.name])
>               except (TypeError, AttributeError):
>                  modlog().warning(value)
        return result

This catchs the following errors in MS CRM 2015 to trap None 'Value' :

WARNING:pyodata.model:{'__metadata': {'type': 'Microsoft.Crm.Sdk.Data.Services.EntityReference'}, 'Id': 'f3d1cca6-0313-e711-80be-005056011e04', 'LogicalName': 'systemuser', 'Name': None} WARNING:pyodata.model:{'__metadata': {'type': 'Microsoft.Crm.Sdk.Data.Services.EntityReference'}, 'Id': None, 'LogicalName': None, 'Name': None} WARNING:pyodata.model:{'__metadata': {'type': 'Microsoft.Crm.Sdk.Data.Services.OptionSetValue'}, 'Value': None}

@filak-sap
Copy link
Contributor

filak-sap commented Apr 21, 2020

Hi @bip91, thank you very much for taking the time to report these issues. I think we will very soon come up with an infrastructure for adding handlers for vendor bugs in OData services since We have already have several discrepancies in ByD services.

Ad 1. This definitely requires some extension point. Will think about a generic solution.

Ad 2. Are you sure it is a bug in MS CRM 2015 and not in pyodata?

@bip91
Copy link
Author

bip91 commented Apr 22, 2020

I definitly agree for the first point, this cannot be solve without an extension as it is a very specific implementation of ODATA. In the meantime, I have set a condition to handle this implementation and will see later.

For second point, I am not enough skilled in Odata protocol to say what should be the correct behaviour. Perhaps examples below could help.

In all cases, pyodata raise a exception on trying to convert to a int a None Value

  • In the 2 first cases, because Value does not exists
  • In the last one, because Value is explicity set to Null

I do not see any side effect on trapping these 2 exceptions and doing nothing, perhaps it is the right thing to do.

**Case 1 and 2 : with 3 samples that raise this exception **
WARNING:pyodata.model:{'__metadata': {'type': 'Microsoft.Crm.Sdk.Data.Services.EntityReference'}, 'Id': 'f3d1cca6-0313-e711-80be-005056011e04', 'LogicalName': 'systemuser', 'Name': None}

WARNING:pyodata.model:{'__metadata': {'type': 'Microsoft.Crm.Sdk.Data.Services.EntityReference'}, 'Id': None, 'LogicalName': None, 'Name': None}

comes from these samples which there is no "Value" tag :

        <d:CreatedBy m:type="Microsoft.Crm.Sdk.Data.Services.EntityReference">
          <d:Id m:type="Edm.Guid">2f804332-197c-e511-80ba-005056011e04</d:Id>
          <d:LogicalName>systemuser</d:LogicalName>
          <d:Name>Service Dynamics CRM</d:Name>
        </d:CreatedBy>

        <d:OwningBusinessUnit m:type="Microsoft.Crm.Sdk.Data.Services.EntityReference">
          <d:Id m:type="Edm.Guid">7dbe6139-8dd1-e711-9105-06b4dc0315b1</d:Id>
          <d:LogicalName>businessunit</d:LogicalName>
          <d:Name m:null="true" />
        </d:OwningBusinessUnit>

        <d:PrimaryContactId m:type="Microsoft.Crm.Sdk.Data.Services.EntityReference">
          <d:Id m:type="Edm.Guid" m:null="true" />
          <d:LogicalName m:null="true" />
          <d:Name m:null="true" />
        </d:PrimaryContactId>

** case 3: is rather the same, but Value is explicitly set to Null **

        <d:OwnershipCode m:type="Microsoft.Crm.Sdk.Data.Services.OptionSetValue">
          <d:Value m:type="Edm.Int32" m:null="true" />
        </d:OwnershipCode>

@filak-sap
Copy link
Contributor

I am afraid that the None/null issue is a bug on pyodata.

In addition to the rules stated in the table, if the value of a primitive type is null, then it is represented as an empty XML element with an m:null="true" attribute ("m" identifies the OData metadata namespace).

https://www.odata.org/documentation/odata-version-2-0/atom-format/

I have to find such a value in Microsoft's Northwind -> https://services.odata.org/V2/Northwind/Northwind.svc/

@filak-sap
Copy link
Contributor

@bip91 Can you re-post the XML responses in JSON here? Just add ?$format=json to the URL.

Example:

The example URLs point the entityset which contains entities having values with null and pyodata can handle them without any problem:

PYTHONPATH=./ bin/pyodata https://services.odata.org/V2/Northwind/Northwind.svc/ ENTITY_SET Customers

the output is the following:

[Initializing HTTP session ...]
[Fetching $metadata ...]
K CustomerID = ALFKI
+ ContactName = Maria Anders
+ PostalCode = 12209
+ City = Berlin
+ Address = Obere Str. 57
+ Country = Germany
+ CompanyName = Alfreds Futterkiste
+ ContactTitle = Sales Representative
+ Region = 
+ Fax = 030-0076545
+ Phone = 030-0074321
--------------------------------------------------------------------------------
...

where the field Region holds null.

@filak-sap
Copy link
Contributor

filak-sap commented Apr 22, 2020

Can you change the lines:

>               except (TypeError, AttributeError):
>                  modlog().warning(value)

to

>               except (TypeError, AttributeError) as ex:
>                  modlog().warning(f'Exception: {ex}')
>                  modlog().warning(f'Property: {type_prop.name}')
>                  modlog().warning(f'Value: {value}')

and repost your results?

@bip91
Copy link
Author

bip91 commented Apr 23, 2020

WARNING:pyodata.model:Exception: int() argument must be a string, a bytes-like object or a number, not 'NoneType'
WARNING:pyodata.model:Property: Value
WARNING:pyodata.model:Value: {'__metadata': {'type': 'Microsoft.Crm.Sdk.Data.Services.OptionSetValue'}, 'Value': None}

WARNING:pyodata.model:Exception: 'NoneType' object has no attribute 'strip'
WARNING:pyodata.model:Property: Name
WARNING:pyodata.model:Value: {'__metadata': {'type': 'Microsoft.Crm.Sdk.Data.Services.EntityReference'}, 'Id': None, 'LogicalName': None, 'Name': None}

WARNING:pyodata.model:Exception: int() argument must be a string, a bytes-like object or a number, not 'NoneType'
WARNING:pyodata.model:Property: Value
WARNING:pyodata.model:Value: {'__metadata': {'type': 'Microsoft.Crm.Sdk.Data.Services.OptionSetValue'}, 'Value': None}

WARNING:pyodata.model:Exception: 'NoneType' object has no attribute 'strip'
WARNING:pyodata.model:Property: Name
WARNING:pyodata.model:Value: {'__metadata': {'type': 'Microsoft.Crm.Sdk.Data.Services.EntityReference'}, 'Id': '7dbe6139-8dd1-e711-9105-06b4dc0315b1', 'LogicalName': 'businessunit', 'Name': None}

It is coming from this kind of values :

  • Aging60 = {'Value': None}
  • OwningTeam = {'Id': None}
  • OwningBusinessUnit = {'Id': '7dbe6139-8dd1-e711-9105-06b4dc0315b1', 'LogicalName': 'businessunit'}

And content in json format :

"Aging60": {
   "__metadata":  { "type": "Microsoft.Crm.Sdk.Data.Services.Money" }, 
   "Value": null
}

"OwningTeam": {
   "__metadata": { "type": "Microsoft.Crm.Sdk.Data.Services.EntityReference" }, 
   "Id": null,  
   "LogicalName": null, 
   "Name": null
}

"OwningBusinessUnit": {
   "__metadata": { "type": "Microsoft.Crm.Sdk.Data.Services.EntityReference" }, 
   "Id": "7dbe6139-8dd1-e711-9105-06b4dc0315b1", 
   "LogicalName": "businessunit", 
   "Name": null
}

@phanak-sap phanak-sap added the bug Something isn't working label Apr 27, 2020
filak-sap added a commit to filak-sap/python-pyodata-1 that referenced this issue Apr 29, 2020
I came a cross this difference during testing of SAP#95

I decided to set tzinfo of JSON DateTimes to simplify my tests and it
also make sense to me to have the values configured the same way.
filak-sap added a commit to filak-sap/python-pyodata-1 that referenced this issue Apr 29, 2020
Only a declared property knows if it accepts Null or not. I was playing
with nullable Type but than I would have to have also nullable
TypeTraits and there result would be a complete mess. Therefore I decided
to move the responsibility to serialize/deserialize the declared
property. The decision led to a simpler code which is a good sign.

I could have used a better exception type but we currently evaluating
the best exception strategies, so I used a generic type.

Part of the issue SAP#95
@filak-sap
Copy link
Contributor

@bip91 Do you think you can test my changes in PR: #100 It only fixes the problem with None.

filak-sap added a commit to filak-sap/python-pyodata-1 that referenced this issue Apr 29, 2020
I came a cross this difference during testing of SAP#95

I decided to set tzinfo of JSON DateTimes to simplify my tests and it
also make sense to me to have the values configured the same way.
filak-sap added a commit to filak-sap/python-pyodata-1 that referenced this issue Apr 29, 2020
Only a declared property knows if it accepts Null or not. I was playing
with nullable Type but than I would have to have also nullable
TypeTraits and there result would be a complete mess. Therefore I decided
to move the responsibility to serialize/deserialize the declared
property. The decision led to a simpler code which is a good sign.

I could have used a better exception type but we currently evaluating
the best exception strategies, so I used a generic type.

Part of the issue SAP#95
@bip91
Copy link
Author

bip91 commented Apr 29, 2020

Now, I get this error on initializing schema :

crm = pyodata.Client(api_url_base,session)
...
INFO:pyodata.client:Creating OData Schema (version: 2)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/zato/tests/python-pyodata-1/pyodata/client.py", line 59, in __new__
    schema = pyodata.v2.model.MetadataBuilder(resp.content, config=config).build()
  File "/opt/zato/tests/python-pyodata-1/pyodata/v2/model.py", line 2451, in build
    raise PyODataParserError(f'Unsupported Schema namespace - {namespace}')
pyodata.exceptions.PyODataParserError: Unsupported Schema namespace - http://schemas.microsoft.com/ado/2007/05/edm

So, I have no initialized context to execute the request.

Please find joined, metadata url provided by server (with some anonymization, I hope that I have not introduce some error with step) :

https://server.com/CRM/XRMServices/2011/OrganizationData.svc/$metadata
content :
$metadata.txt

I hope this could help.

@filak-sap
Copy link
Contributor

I cannot reproduce the problem. Didn't you touch this list:
https://github.com/SAP/python-pyodata/blob/master/pyodata/v2/model.py#L2441

@filak-sap
Copy link
Contributor

In my version of pyodata, the exception you got is raised at the line 2535 but in your case it's line 2451.

@bip91
Copy link
Author

bip91 commented Apr 29, 2020

Sorry for this erroneous first feedback, with a fully new fresh installation it works like a charm.
Patch is working for me.

@filak-sap filak-sap assigned phanak-sap and unassigned phanak-sap Apr 29, 2020
filak-sap added a commit to filak-sap/python-pyodata-1 that referenced this issue Apr 29, 2020
I came a cross this difference during testing of SAP#95

I decided to set tzinfo of JSON DateTimes to simplify my tests and it
also make sense to me to have the values configured the same way.
filak-sap added a commit to filak-sap/python-pyodata-1 that referenced this issue Apr 29, 2020
Only a declared property knows if it accepts Null or not. I was playing
with nullable Type but than I would have to have also nullable
TypeTraits and there result would be a complete mess. Therefore I decided
to move the responsibility to serialize/deserialize the declared
property. The decision led to a simpler code which is a good sign.

I could have used a better exception type but we currently evaluating
the best exception strategies, so I used a generic type.

Part of the issue SAP#95
filak-sap added a commit to filak-sap/python-pyodata-1 that referenced this issue Apr 30, 2020
I came a cross this difference during testing of SAP#95

I decided to set tzinfo of JSON DateTimes to simplify my tests and it
also make sense to me to have the values configured the same way.
filak-sap added a commit to filak-sap/python-pyodata-1 that referenced this issue Apr 30, 2020
Only a declared property knows if it accepts Null or not. I was playing
with nullable Type but than I would have to have also nullable
TypeTraits and there result would be a complete mess. Therefore I decided
to move the responsibility to serialize/deserialize the declared
property. The decision led to a simpler code which is a good sign.

I could have used a better exception type but we currently evaluating
the best exception strategies, so I used a generic type.

Part of the issue SAP#95
filak-sap added a commit that referenced this issue Apr 30, 2020
I came a cross this difference during testing of #95

I decided to set tzinfo of JSON DateTimes to simplify my tests and it
also make sense to me to have the values configured the same way.
filak-sap added a commit that referenced this issue Apr 30, 2020
Only a declared property knows if it accepts Null or not. I was playing
with nullable Type but than I would have to have also nullable
TypeTraits and there result would be a complete mess. Therefore I decided
to move the responsibility to serialize/deserialize the declared
property. The decision led to a simpler code which is a good sign.

I could have used a better exception type but we currently evaluating
the best exception strategies, so I used a generic type.

Part of the issue #95
@phanak-sap
Copy link
Contributor

phanak-sap commented May 5, 2020

After merging of #102 this issue seems to be ready to be closed.
@bip91 could you confirm that latest head 7972f31 works for you?

I will create release 1.5.0 soon.

@filak-sap
Copy link
Contributor

@bip91 It would help us if you create a new issue for the PATCH vs. MERGE request.

@bip91
Copy link
Author

bip91 commented May 7, 2020

Latest head works perfectly, I have deployed it instead of my first workaround.

I have also isolated the part "MERGE in a vendor request" in #103

@bip91 bip91 closed this as completed Jun 30, 2020
@jfilak
Copy link
Contributor

jfilak commented Jun 30, 2020

@bip91 I would not mind if you create a new factory method MS_CRM_client in the vendor module which would return pyodata client correctly configured.

@bip91 bip91 reopened this Jun 30, 2020
@bip91
Copy link
Author

bip91 commented Jun 30, 2020

Yes, of course.

I have built MSCRM vendor extension :
MSCRM.py.txt

This works for me with the following code :

import vendor.MSCRM
crm = vendor.MSCRM.Client(api_url_base,session)

# example to set a on guid id
crm.entity_sets.ContactSet.update_entity(id).set(**a).execute()

The endpoint has changed on more recent Dynamics CRM so the test on URL should be sufficient to avoid mistakes, but I cannot validate this (I have only Dynamics CRM 2015)

Feel free to adjust my extension to be coherent with the others.

IvanShirokikh pushed a commit to RYDLAB/python-pyodata that referenced this issue Oct 3, 2020
I came a cross this difference during testing of SAP#95

I decided to set tzinfo of JSON DateTimes to simplify my tests and it
also make sense to me to have the values configured the same way.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants