CartoDB / carto-python

CARTO Python client
https://carto.com
BSD 3-Clause "New" or "Revised" License
154 stars 62 forks source link

Include synchronization data in the dataset model #98

Closed ethervoid closed 5 years ago

ethervoid commented 5 years ago

Related https://github.com/CartoDB/carto-python/issues/96

Sometimes we want to know if a datasets come is synchronized so we need to add that information when we ask for the dataset/s

ethervoid commented 5 years ago

@alrocar / @danicarrion any reason to no include sync data in the dataset?

danicarrion commented 5 years ago

Not that I'm aware of. Most likely I didn't know what that parameter meant at that time and didn't include it.

ethervoid commented 5 years ago

@alrocar adding it as Charfield(many=true) works just fine, adding all the fields as a dict

In [2]: dsets = dm.all()                                                                                                                                                                                                                
/media/data/development/cartodb/carto-python/carto/resources.py:90: FutureWarning: This is part of a non-public CARTO API and may change in the future. Take this into account if you are using this in a production environment
  warnings.warn('This is part of a non-public CARTO API and may change in the future. Take this into account if you are using this in a production environment', FutureWarning)

In [3]: dsets[8].synchronization                                                                                                                                                                                                        
Out[3]: 
{'checksum': None,
 'created_at': '2018-08-02T07:48:50.118Z',
 'error_code': None,
 'error_message': None,
 'id': '7e921cb6-9628-11e8-a1ef-000d3a2833f8',
 'interval': 2592000,
 'modified_at': '2018-06-21T16:58:28.000Z',
 'name': 'us_state_1860',
 'ran_at': '2018-10-31T07:51:34.411Z',
 'retried_times': 0,
 'run_at': '2018-11-30T07:51:34.430Z',
 'service_item_id': 'https://common-data.cartodb.com/api/v2/sql?q=select+*+from+%22us_state_1860%22&format=shp&filename=us_state_1860',
 'service_name': None,
 'state': 'success',
 'updated_at': '2018-10-31T07:51:34.474Z',
 'url': 'https://common-data.cartodb.com/api/v2/sql?q=select+*+from+%22us_state_1860%22&format=shp&filename=us_state_1860',
 'user_id': '5294a959-4345-4316-a982-f12161b7838b',
 'content_guessing': True,
 'etag': None,
 'log_id': '07bd6d66-454c-4cbb-9a85-0b691fbf427c',
 'quoted_fields_guessing': True,
 'type_guessing': True,
 'from_external_source': True,
 'visualization_id': '8bdc2c40-3aa9-42c1-8b71-85c0073ee10c'}

In [4]: dsets[8].synchronization['checksum']                                                                                                                                                                                            

In [5]: dsets[8].synchronization.__class__                                                                                                                                                                                              
Out[5]: dict

In [6]: dsets[8].synchronization['id']                                                                                                                                                                                                  
Out[6]: '7e921cb6-9628-11e8-a1ef-000d3a2833f8'

In [7]: dsets[8].synchronization['checksum']                                                                                                                                                                                            

In [8]: dsets[8].synchronization['ran_at']                                                                                                                                                                                              
Out[8]: '2018-10-31T07:51:34.411Z'

In [9]: dsets[8].synchronization['user_id']                                                                                                                                                                                             
Out[9]: '5294a959-4345-4316-a982-f12161b7838b'

In [10]:    

But if the way to do this is to add a new kind of type like you did in that PR, I'll gladly add it and change my PR

ethervoid commented 5 years ago

Added

In [2]: dsets = dm.all()                                                                                                                                                                                                                
/media/data/development/cartodb/carto-python/carto/resources.py:90: FutureWarning: This is part of a non-public CARTO API and may change in the future. Take this into account if you are using this in a production environment
  warnings.warn('This is part of a non-public CARTO API and may change in the future. Take this into account if you are using this in a production environment', FutureWarning)

In [3]: dsets[8].synchronization                                                                                                                                                                                                        
Out[3]: <carto.synchronizations.Synchronization at 0x7fd2050128d0>

In [4]: dsets[8].synchronization.fields                                                                                                                                                                                                 
Out[4]: 
['checksum',
 'created_at',
 'error_code',
 'error_message',
 'id',
 'interval',
 'modified_at',
 'name',
 'ran_at',
 'retried_times',
 'run_at',
 'service_item_id',
 'service_name',
 'state',
 'updated_at',
 'url',
 'user_id',
 'content_guessing',
 'etag',
 'log_id',
 'quoted_fields_guessing',
 'type_guessing',
 'from_external_source',
 'visualization_id']

In [5]: dsets[8].synchronization.checksum                                                                                                                                                                                               

In [6]: dsets[8].synchronization.id                                                                                                                                                                                                     
Out[6]: '7e921cb6-9628-11e8-a1ef-000d3a2833f8'

In [7]: dsets[8].synchronization.state                                                                                                                                                                                                  
Out[7]: 'success'
ethervoid commented 5 years ago

Also added in Visualization

In [5]: visualization = visualization_manager.get("us_state_1860")                                                                                                                                                                      
/media/data/development/cartodb/carto-python/carto/resources.py:90: FutureWarning: This is part of a non-public CARTO API and may change in the future. Take this into account if you are using this in a production environment
  warnings.warn('This is part of a non-public CARTO API and may change in the future. Take this into account if you are using this in a production environment', FutureWarning)

In [6]: visualization.synchronization                                                                                                                                                                                                   
Out[6]: <carto.synchronizations.Synchronization at 0x7f90d2b4f400>

In [7]: visualization.synchronization.fields                                                                                                                                                                                            
Out[7]: 
['checksum',
 'created_at',
 'error_code',
 'error_message',
 'id',
 'interval',
 'modified_at',
 'name',
 'ran_at',
 'retried_times',
 'run_at',
 'service_item_id',
 'service_name',
 'state',
 'updated_at',
 'url',
 'user_id',
 'content_guessing',
 'etag',
 'log_id',
 'quoted_fields_guessing',
 'type_guessing',
 'from_external_source',
 'visualization_id']

In [8]: visualization.synchronization.state                                                                                                                                                                                             
Out[8]: 'success'
javitonino commented 5 years ago

As a comment, I'm not a fan of adding everything returned by the endpoint directly. This is using an undocument/unsupported endpoint, so we reserve the change it without notice (and are doing so right now) and it could break user code.

I'd try restricting the returned value to a smaller subset that makes sense for the final user. For example, the final user doesn't care about internal things like checksum or etag and in fact those may disappear from the API without notice.

That said, we used the same approach for datasets, so we may need to break support at some point anyway :cry:

alrocar commented 5 years ago

Yep, non-public APIs raise a warning in the first place when you use them.

If you know which attributes would not be necessary or we are changing we can remove them from the response otherwise the PR looks good to me.

ethervoid commented 5 years ago

@alrocar added WarnAsynResource parent class for the sync one in order to show warning message if is used. Second round?