Looks good! +1 with a couple of refactoring/renaming comments:
[1]
I feel the term "cloud" is a little overused everywhere, making things less obvious that they could be. In the future I suppose we will add meta data for more than just EC2, so how about:
- Renaming _fetch_cloud_meta_data to _fetch_and_log_ec2_data (or similar) as this is really an EC2 specific method.
- Changing the logs to say "EC2" instead of "Cloud" in the aforementioned method
[2]
I would like to keep the message content gathering and creation logic in _create_computer_info_message. This would simply mean moving
+ if (self._cloud_meta_data is None and
+ self._cloud_retries < METADATA_RETRY_MAX):
+ self._cloud_meta_data = yield self._fetch_cloud_meta_data()
to _create_computer_info_message, and marking _create_computer_info_message as @inlineCallback instead of send_computer_message
If this is done, I'm not certain it's useful to keep self._cloud_meta_data (maybe a local variable is sufficient inside _create_computer_info_message)
Then in the future we can then easily extend the functionality by adding _fetch_and_log_<whatever>_data() methods to the class and hooking it into _create_computer_info_message.
Looks good! +1 with a couple of refactoring/ renaming comments:
[1] cloud_meta_ data to _fetch_ and_log_ ec2_data (or similar) as this is really an EC2 specific method.
I feel the term "cloud" is a little overused everywhere, making things less obvious that they could be. In the future I suppose we will add meta data for more than just EC2, so how about:
- Renaming _fetch_
- Changing the logs to say "EC2" instead of "Cloud" in the aforementioned method
[2] computer_ info_message. This would simply mean moving
I would like to keep the message content gathering and creation logic in _create_
+ if (self._ cloud_meta_ data is None and RETRY_MAX) : meta_data = yield self._fetch_ cloud_meta_ data()
+ self._cloud_retries < METADATA_
+ self._cloud_
to _create_ computer_ info_message, and marking _create_ computer_ info_message as @inlineCallback instead of send_computer_ message
If this is done, I'm not certain it's useful to keep self._cloud_ meta_data (maybe a local variable is sufficient inside _create_ computer_ info_message)
Then in the future we can then easily extend the functionality by adding _fetch_ and_log_ <whatever> _data() methods to the class and hooking it into _create_ computer_ info_message.