libcloud-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [libcloud] macfreek edited a comment on pull request #1537: Deprecate Record.ttl in favour of Record.extra['ttl']
Date Wed, 30 Dec 2020 00:37:46 GMT

macfreek edited a comment on pull request #1537:
URL: https://github.com/apache/libcloud/pull/1537#issuecomment-752286927


   This seems to open a little can of worms in the unit tests.
   
   There are now 3 failing unit tests, most of which are caused by 
   The following excerpts are from the current unit tests. Please carefully check if this
is really what is intended:
   
   ## LuaDNS
   
   TTL is set to 13, but is [expected to be None when read](https://github.com/apache/libcloud/blob/6b588346083a510396c63b5d18db3012aa083071/libcloud/test/dns/test_luadns.py#L202):
   
       def test_create_record_success(self):
           LuadnsMockHttp.type = 'CREATE_RECORD_SUCCESS'
           record = self.driver.create_record(name='test.com.',
                                              zone=self.test_zone,
                                              type='A',
                                              data='127.0.0.1',
                                              extra={'ttl': 13})
           # ...
           self.assertIsNone(record.ttl)
   
   ## DNSPod
   
   The behaviour of DNSPod seems a bit erratic to me. See #1538 for details.
   
   The following unit test now fails:
   
       def test_create_record_success(self):
           DNSPodMockHttp.type = 'CREATE_RECORD_SUCCESS'
           record = self.driver.create_record(name='@', zone=self.test_zone,
                                              type='A', data='96.126.115.73',
                                              extra={'ttl': 13,
                                                     'record_line': 'default'})
   
   Old behaviour
   ```
   record.ttl == None
   record.extra['ttl'] == '600'
   ```
   
   New behaviour
   ```
   record.ttl == '600'
   record.extra['ttl'] == '600'
   ```
   
   Where `'600'` is the default TTL for the zone. The extra parameter (with `ttl: 13`) is
completely ignored. This PR exposes this erratic behaviour. Hence, the failed unit test. I'm
hesitant to fix this particular bug in this PR (since I'm not familiar how the DNSPod expects
the TTL), but I don't want to remove the unit test either.
   
   ## DurableDNS (now fixed)
   
   DurableDNSTests.test_list_records() fails, which exposes a minor bug in this PR (which
is subsequently fixed).
   
   DurableDNSTest set TTL = 3600, then reads the record, which returns '3600'.
   The DurableDNSDriver casts this to int for record.extra['ttl'], but not for record.ttl.
   
   Previous behaviour:
   ```
   record.ttl == '3600'
   record.extra['ttl'] == 3600
   ```
   
   Original PR:
   ```
   record.ttl == '3600'
   record.extra['ttl'] == '3600'
   ```
   
   Final PR:
   ```
   record.ttl == 3600
   record.extra['ttl'] == 3600
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



Mime
View raw message