Change method of reporting target data #4593

Merged
merged 11 commits into from May 18, 2017

Conversation

Projects
None yet
3 participants
@dotordogh
Contributor

dotordogh commented May 15, 2017

Problem

In a previous version we were reporting target information by providing a string that is a key and a value which was a dict containing a lot of information. We want to be able to provide a list of keys that will recursively find the right place in a nested dict to store a primitive value (rather than a dictionary) to the reporting method.

Solution

I created to helper methods to do this work. One recursively nests they keys with the value, and another does the work of finding the correctly place in an existing dictionary to update with the new nested dict.

Result

These changes will affect how developers call the report_target_info API. As far as I know, no one is using it yet, so we don't need to make any changes to the dev documentation (apart from the method docs itself).

Dorothy Ordogh added some commits May 15, 2017

Dorothy Ordogh
Add method that creates a nested dict from a list of keys and a metho…
…d that updates the dict appropriately given a list of keys
Dorothy Ordogh
Edit report test info method so that it takes a list of keys rather t…
…han just a string for test name and update junit and pytest runners to use it accordingly
@stuhood

Thanks Dorothy... the tests look thorough. It looks like it might be possible to simplify the logic a bit... it's not clear why slicing is necessary.

src/python/pants/goal/run_tracker.py
+ new_data_to_add = RunTracker.create_dict_with_nested_keys_and_val(keys, value, len(keys) - 1)
+ data.update(new_data_to_add)
+ return
+ else:

This comment has been minimized.

@stuhood

stuhood May 15, 2017

Member

Since both of the if statements result in the function terminating they can be treated as guard clauses, and the else here isn't necessary: can remove the else and dedent its body.

(opinions on guard clauses are mixed: but my take is that if they reduce nesting, they're a good idea.)

@stuhood

stuhood May 15, 2017

Member

Since both of the if statements result in the function terminating they can be treated as guard clauses, and the else here isn't necessary: can remove the else and dedent its body.

(opinions on guard clauses are mixed: but my take is that if they reduce nesting, they're a good idea.)

This comment has been minimized.

@dotordogh

dotordogh May 16, 2017

Contributor

Good idea!

@dotordogh

dotordogh May 16, 2017

Contributor

Good idea!

src/python/pants/goal/run_tracker.py
+ Will recursively find the correct place to store in the nested dicts.
+ :param primitive value: The value of the information being stored.
+ :param int index: The index into the list of keys (starting form the beginning). This should
+ always be 0 when being called outside of this function.

This comment has been minimized.

@stuhood

stuhood May 15, 2017

Member

Could probably default it to 0 then?

@stuhood

stuhood May 15, 2017

Member

Could probably default it to 0 then?

This comment has been minimized.

@dotordogh

dotordogh May 16, 2017

Contributor

Right! Done!

@dotordogh

dotordogh May 16, 2017

Contributor

Right! Done!

src/python/pants/goal/run_tracker.py
+ RunTracker.merge_list_of_keys_into_dict(this_keys_contents, new_keys, value, 0)
+ else:
+ new_keys = keys[index+1:]
+ new_data_to_add = RunTracker.create_dict_with_nested_keys_and_val(new_keys, value, len(new_keys) - 1)

This comment has been minimized.

@stuhood

stuhood May 15, 2017

Member

I'm a bit confused by the usage of index here. What I'm expecting to see is that this looks like a recursive loop, ie: the index always starts at 0 and then increments to the initial value (from the first call) of len(keys).

In that situation, it would not be necessary to slice (keys[index+1:]) on the keys, because the list of keys would be the same the entire time, and only the index would change.

@stuhood

stuhood May 15, 2017

Member

I'm a bit confused by the usage of index here. What I'm expecting to see is that this looks like a recursive loop, ie: the index always starts at 0 and then increments to the initial value (from the first call) of len(keys).

In that situation, it would not be necessary to slice (keys[index+1:]) on the keys, because the list of keys would be the same the entire time, and only the index would change.

This comment has been minimized.

@dotordogh

dotordogh May 16, 2017

Contributor

I was slicing the the arrays because it simplifies the create_dict_with_nested_keys_and_val method. Since it that method starts from the end of the list and works forward, if we slice the array, we don't need to worry about passing in another integer that is the highest key we want in the nested dict.

But you are totally right that the recursive call to merge_list_of_keys_into_dict should just increase the index and not slice the keys.

@dotordogh

dotordogh May 16, 2017

Contributor

I was slicing the the arrays because it simplifies the create_dict_with_nested_keys_and_val method. Since it that method starts from the end of the list and works forward, if we slice the array, we don't need to worry about passing in another integer that is the highest key we want in the nested dict.

But you are totally right that the recursive call to merge_list_of_keys_into_dict should just increase the index and not slice the keys.

src/python/pants/goal/run_tracker.py
+ else:
+ this_keys_contents = data.get(keys[index])
+ if this_keys_contents:
+ if isinstance(this_keys_contents, dict):

This comment has been minimized.

@stuhood

stuhood May 15, 2017

Member

I think it would be appropriate to explode if you have multiple remaining keys and the existing data is not a dict.

@stuhood

stuhood May 15, 2017

Member

I think it would be appropriate to explode if you have multiple remaining keys and the existing data is not a dict.

This comment has been minimized.

@dotordogh

dotordogh May 16, 2017

Contributor

You don't think we should overwrite the data if it is a primitive? I was thinking if we had a dict {'a': {'b': 'data'}} and we want to write with the keys ['a', 'b', 'c', 'd'] and some value 'data', the result be {'a': {'b': {'c': {'d': 'data'}}}}. Is that assumption wrong?

@dotordogh

dotordogh May 16, 2017

Contributor

You don't think we should overwrite the data if it is a primitive? I was thinking if we had a dict {'a': {'b': 'data'}} and we want to write with the keys ['a', 'b', 'c', 'd'] and some value 'data', the result be {'a': {'b': {'c': {'d': 'data'}}}}. Is that assumption wrong?

This comment has been minimized.

@stuhood

stuhood May 16, 2017

Member

I think it would be reasonable to explode in that case... I think the expected usage of this API is that that would indicate a bug in a single task. The expectation is that a single task will probably be writing within a scope (with the exception of the 'GLOBAL' scope), and so a task overwriting its own data would almost certainly be a bug.

@stuhood

stuhood May 16, 2017

Member

I think it would be reasonable to explode in that case... I think the expected usage of this API is that that would indicate a bug in a single task. The expectation is that a single task will probably be writing within a scope (with the exception of the 'GLOBAL' scope), and so a task overwriting its own data would almost certainly be a bug.

This comment has been minimized.

@dotordogh

dotordogh May 16, 2017

Contributor

@stuhood I changed the code so that it would blow up when writing to a non-dictionary, but I forgot that it will blow up if we try to overwrite a value that already exists in the dict. I'm thinking, rather than throwing an error, we could add a debugging statement that includes the keys and value saying a value can't be overwritten? Thoughts?

Or we can just remove the else statement entirely. I've noticed this behavior when the same test target is run multiple times in one pants run or when we don't check that the keys and value already exists in the dictionary. I think it might be better to remove the else statement entirely. That way we aren't adding a bunch of log statements and seemingly have no use, and we can just state in the docs that once a set of keys and a value are written, they cannot be changed.

@dotordogh

dotordogh May 16, 2017

Contributor

@stuhood I changed the code so that it would blow up when writing to a non-dictionary, but I forgot that it will blow up if we try to overwrite a value that already exists in the dict. I'm thinking, rather than throwing an error, we could add a debugging statement that includes the keys and value saying a value can't be overwritten? Thoughts?

Or we can just remove the else statement entirely. I've noticed this behavior when the same test target is run multiple times in one pants run or when we don't check that the keys and value already exists in the dictionary. I think it might be better to remove the else statement entirely. That way we aren't adding a bunch of log statements and seemingly have no use, and we can just state in the docs that once a set of keys and a value are written, they cannot be changed.

@dotordogh

Thank you, Stu! I left replies :)

src/python/pants/goal/run_tracker.py
+ RunTracker.merge_list_of_keys_into_dict(this_keys_contents, new_keys, value, 0)
+ else:
+ new_keys = keys[index+1:]
+ new_data_to_add = RunTracker.create_dict_with_nested_keys_and_val(new_keys, value, len(new_keys) - 1)

This comment has been minimized.

@dotordogh

dotordogh May 16, 2017

Contributor

I was slicing the the arrays because it simplifies the create_dict_with_nested_keys_and_val method. Since it that method starts from the end of the list and works forward, if we slice the array, we don't need to worry about passing in another integer that is the highest key we want in the nested dict.

But you are totally right that the recursive call to merge_list_of_keys_into_dict should just increase the index and not slice the keys.

@dotordogh

dotordogh May 16, 2017

Contributor

I was slicing the the arrays because it simplifies the create_dict_with_nested_keys_and_val method. Since it that method starts from the end of the list and works forward, if we slice the array, we don't need to worry about passing in another integer that is the highest key we want in the nested dict.

But you are totally right that the recursive call to merge_list_of_keys_into_dict should just increase the index and not slice the keys.

src/python/pants/goal/run_tracker.py
+ else:
+ this_keys_contents = data.get(keys[index])
+ if this_keys_contents:
+ if isinstance(this_keys_contents, dict):

This comment has been minimized.

@dotordogh

dotordogh May 16, 2017

Contributor

You don't think we should overwrite the data if it is a primitive? I was thinking if we had a dict {'a': {'b': 'data'}} and we want to write with the keys ['a', 'b', 'c', 'd'] and some value 'data', the result be {'a': {'b': {'c': {'d': 'data'}}}}. Is that assumption wrong?

@dotordogh

dotordogh May 16, 2017

Contributor

You don't think we should overwrite the data if it is a primitive? I was thinking if we had a dict {'a': {'b': 'data'}} and we want to write with the keys ['a', 'b', 'c', 'd'] and some value 'data', the result be {'a': {'b': {'c': {'d': 'data'}}}}. Is that assumption wrong?

src/python/pants/goal/run_tracker.py
+ Will recursively find the correct place to store in the nested dicts.
+ :param primitive value: The value of the information being stored.
+ :param int index: The index into the list of keys (starting form the beginning). This should
+ always be 0 when being called outside of this function.

This comment has been minimized.

@dotordogh

dotordogh May 16, 2017

Contributor

Right! Done!

@dotordogh

dotordogh May 16, 2017

Contributor

Right! Done!

src/python/pants/goal/run_tracker.py
+ new_data_to_add = RunTracker.create_dict_with_nested_keys_and_val(keys, value, len(keys) - 1)
+ data.update(new_data_to_add)
+ return
+ else:

This comment has been minimized.

@dotordogh

dotordogh May 16, 2017

Contributor

Good idea!

@dotordogh

dotordogh May 16, 2017

Contributor

Good idea!

@stuhood

Thanks Dorothy. Aside from adding an Exception for trying to overwrite an existing dict with a primitive, this looks good.

Benjy is back tomorrow, so let's wait for his go before merging.

src/python/pants/goal/run_tracker.py
+ new_data_to_add = RunTracker.create_dict_with_nested_keys_and_val(new_keys, value,
+ len(new_keys) - 1)
+ data.update(new_data_to_add)
+ return

This comment has been minimized.

@stuhood

stuhood May 16, 2017

Member

Redundant statement... since the function doesn't return anything, no need to explicitly return.

@stuhood

stuhood May 16, 2017

Member

Redundant statement... since the function doesn't return anything, no need to explicitly return.

- self.report_test_info(self.options_scope, test_target, test_name, test_info)
+ for test_info_key, test_info_val in test_info.items():
+ key_list = [test_name, test_info_key]
+ self.report_test_info(self.options_scope, test_target, key_list, test_info_val)

This comment has been minimized.

@stuhood

stuhood May 16, 2017

Member

Given that the only user of this API is now using the "key list" API, would it be reasonable to remove the support for passing in and merging dicts? I know you spent some time on that, but this seems less error prone, etc. Up to you.

@stuhood

stuhood May 16, 2017

Member

Given that the only user of this API is now using the "key list" API, would it be reasonable to remove the support for passing in and merging dicts? I know you spent some time on that, but this seems less error prone, etc. Up to you.

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

I second any suggestion to keep things no more complicated than they need to be to support current use cases. :)

@benjyw

benjyw May 18, 2017

Contributor

I second any suggestion to keep things no more complicated than they need to be to support current use cases. :)

This comment has been minimized.

@dotordogh

dotordogh May 18, 2017

Contributor

It has already been removed :)

@dotordogh

dotordogh May 18, 2017

Contributor

It has already been removed :)

src/python/pants/goal/run_tracker.py
+ def merge_list_of_keys_into_dict(data, keys, value, index=0):
+ """Recursively merge list of keys that points to the given value into data.
+
+ Will not overwrite data that is already associated with a given key list.

This comment has been minimized.

@stuhood

stuhood May 16, 2017

Member

Should comment on what it will do instead.

@stuhood

stuhood May 16, 2017

Member

Should comment on what it will do instead.

src/python/pants/goal/run_tracker.py
:param dict data: Dictionary to be updated.
:param list of string keys: The keys that point to where the value should be stored.
Will recursively find the correct place to store in the nested dicts.
:param primitive value: The value of the information being stored.
- :param int index: The index into the list of keys (starting form the beginning). This should
- always be 0 when being called outside of this function.
+ :param int index: The index into the list of keys (starting form the beginning).

This comment has been minimized.

@stuhood

stuhood May 16, 2017

Member

"starting from"

@stuhood

stuhood May 16, 2017

Member

"starting from"

src/python/pants/goal/run_tracker.py
:param dict data: Dictionary to be updated.
:param list of string keys: The keys that point to where the value should be stored.
Will recursively find the correct place to store in the nested dicts.
:param primitive value: The value of the information being stored.
- :param int index: The index into the list of keys (starting form the beginning). This should
- always be 0 when being called outside of this function.
+ :param int index: The index into the list of keys (starting form the beginning).
"""
if len(keys) == 0:
raise ValueError('Keys must contain at least one key')
if len(keys) < 2 or not data:

This comment has been minimized.

@stuhood

stuhood May 16, 2017

Member

Should this clause take the index into account?

@stuhood

stuhood May 16, 2017

Member

Should this clause take the index into account?

src/python/pants/goal/run_tracker.py
+
+ this_keys_contents = data.get(keys[index])
+ if this_keys_contents:
+ if isinstance(this_keys_contents, dict):

This comment has been minimized.

@stuhood

stuhood May 16, 2017

Member

If this condition isn't true, this should likely explode with an exception. Silently dropping data isn't great.

@stuhood

stuhood May 16, 2017

Member

If this condition isn't true, this should likely explode with an exception. Silently dropping data isn't great.

@stuhood

This comment has been minimized.

Show comment
Hide comment
@stuhood

stuhood May 16, 2017

Member

Hm... also: note that my most recent comments are accidentally pre-collapsed because I was commenting on your most recent diff.

Member

stuhood commented May 16, 2017

Hm... also: note that my most recent comments are accidentally pre-collapsed because I was commenting on your most recent diff.

@benjyw

Overall looks good - see comments inline.

Thanks for breaking this up into two changes. Makes it easier to review.

@@ -434,7 +434,9 @@ def parse_error_handler(parse_error):
for test_name, test_info in tests_info.items():
test_item = Test(test_info['classname'], test_name)
test_target = test_registry.get_owning_target(test_item)
- self.report_test_info(self.options_scope, test_target, test_name, test_info)
+ for test_info_key, test_info_val in test_info.items():

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

Could this logic go into report_test_info() (or even better, a function that wraps it), instead of being repeated here and in pytest_run.py?

@benjyw

benjyw May 18, 2017

Contributor

Could this logic go into report_test_info() (or even better, a function that wraps it), instead of being repeated here and in pytest_run.py?

src/python/pants/goal/run_tracker.py
+ @staticmethod
+ def create_dict_with_nested_keys_and_val(all_keys, value, index):
+ """Recursively constructs a nested dictionary with the keys pointing to the value.
+

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

It would be super-helpful to have an example here.

@benjyw

benjyw May 18, 2017

Contributor

It would be super-helpful to have an example here.

src/python/pants/goal/run_tracker.py
+ """
+ if index > 0:
+ new_val = {all_keys[index]: value}
+ return RunTracker.create_dict_with_nested_keys_and_val(all_keys, new_val, index - 1)

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

If you make this a @classmethod (whose first param is then cls - the class) instead of a @staticmethod, then this call can be cls._create_dict_with_nested_keys_and_val(...), which is slightly neater than repeating the class name here.

@benjyw

benjyw May 18, 2017

Contributor

If you make this a @classmethod (whose first param is then cls - the class) instead of a @staticmethod, then this call can be cls._create_dict_with_nested_keys_and_val(...), which is slightly neater than repeating the class name here.

src/python/pants/goal/run_tracker.py
+ return {all_keys[index]: value}
+
+ @staticmethod
+ def merge_list_of_keys_into_dict(data, keys, value, index=0):

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

Again, can start with an underscore.

@benjyw

benjyw May 18, 2017

Contributor

Again, can start with an underscore.

src/python/pants/goal/run_tracker.py
@@ -414,21 +414,77 @@ def shutdown_worker_pool(self):
"""
SubprocPool.shutdown(self._aborted)
- def report_target_info(self, scope, target, key, val):
+ @staticmethod
+ def create_dict_with_nested_keys_and_val(all_keys, value, index):

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

Since this is an internal helper, not intended to be used outside this class, it's good practice to have the name start with an underscore (_create_dict_with_nested_keys_and_val). This is idiomatic python for "this is a 'private' method".

@benjyw

benjyw May 18, 2017

Contributor

Since this is an internal helper, not intended to be used outside this class, it's good practice to have the name start with an underscore (_create_dict_with_nested_keys_and_val). This is idiomatic python for "this is a 'private' method".

+
+ Will override a primitive value with another primitive value, but will not
+ override a primitive with a dictionary.
+

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

Again, an example here would prove very helpful!

@benjyw

benjyw May 18, 2017

Contributor

Again, an example here would prove very helpful!

src/python/pants/goal/run_tracker.py
- scope_data.update({key: val})
+ new_key_list = [target, scope]
+ new_key_list += keys
+ RunTracker.merge_list_of_keys_into_dict(self._target_to_data, new_key_list, val, 0)

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

This can be self._merge_list_of_keys_into_dict.

@benjyw

benjyw May 18, 2017

Contributor

This can be self._merge_list_of_keys_into_dict.

@@ -57,3 +57,115 @@ def test_write_stats_to_json_file(self):
with open(file_name) as f:
result = json.load(f)
self.assertEquals(stats, result)
+
+ def test_create_dict_with_nested_keys_and_val(self):

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

Thanks for the comprehensive tests!

@benjyw

benjyw May 18, 2017

Contributor

Thanks for the comprehensive tests!

- self.report_test_info(self.options_scope, test_target, test_name, test_info)
+ for test_info_key, test_info_val in test_info.items():
+ key_list = [test_name, test_info_key]
+ self.report_test_info(self.options_scope, test_target, key_list, test_info_val)

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

I second any suggestion to keep things no more complicated than they need to be to support current use cases. :)

@benjyw

benjyw May 18, 2017

Contributor

I second any suggestion to keep things no more complicated than they need to be to support current use cases. :)

src/python/pants/goal/run_tracker.py
+
+ :param list of string all_keys: A list of keys to be nested as a dictionary.
+ :param dict or string value: The value of the information being stored.
+ :param int index: The index of the last element of the list of keys.

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

Instead of passing in a list and an index, why not pass in a truncated copy of the list? The code will be more readable, and the performance implications are almost certainly unmeasurable.

@benjyw

benjyw May 18, 2017

Contributor

Instead of passing in a list and an index, why not pass in a truncated copy of the list? The code will be more readable, and the performance implications are almost certainly unmeasurable.

@benjyw

Very close now. Just a handful of remaining comments.

+ for test_info_key, test_info_val in test_info.items():
+ key_list = [test_name, test_info_key]
+ self.report_test_info(scope, target, key_list, test_info_val)
+
def report_test_info(self, scope, target, keys, test_info):

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

Can this now be _private?

@benjyw

benjyw May 18, 2017

Contributor

Can this now be _private?

src/python/pants/goal/run_tracker.py
+ current_index = len(keys) - 1
+
+ if len(keys) > 1:
+ new_val = {keys[current_index]: value}

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

You can use keys[-1] here...

@benjyw

benjyw May 18, 2017

Contributor

You can use keys[-1] here...

src/python/pants/goal/run_tracker.py
+
+ if len(keys) > 1:
+ new_val = {keys[current_index]: value}
+ new_keys = keys[:current_index]

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

and keys[:-1] here...

@benjyw

benjyw May 18, 2017

Contributor

and keys[:-1] here...

src/python/pants/goal/run_tracker.py
+ new_keys = keys[:current_index]
+ return cls._create_dict_with_nested_keys_and_val(new_keys, new_val)
+ elif len(keys) == 1:
+ return {keys[current_index]: value}

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

and keys[0] here, and thus get rid of current_index entirely!

Sometimes Python can be pretty neat!

@benjyw

benjyw May 18, 2017

Contributor

and keys[0] here, and thus get rid of current_index entirely!

Sometimes Python can be pretty neat!

src/python/pants/goal/run_tracker.py
+ return cls._create_dict_with_nested_keys_and_val(new_keys, new_val)
+ elif len(keys) == 1:
+ return {keys[current_index]: value}
+

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

It's a bit unorthodox to have a function (especially a recursive function) that returns a value but that doesn't handle every possible branch. In this case, len(keys) == 0 is not handled. Effictively it's like returning None, which might cause an unintuitive error down the line. So it's better to throw an explicit error as soon as this case is encountered.

@benjyw

benjyw May 18, 2017

Contributor

It's a bit unorthodox to have a function (especially a recursive function) that returns a value but that doesn't handle every possible branch. In this case, len(keys) == 0 is not handled. Effictively it's like returning None, which might cause an unintuitive error down the line. So it's better to throw an explicit error as soon as this case is encountered.

Dorothy Ordogh
make report_test_info private, remove current_index variable, and add…
… final else case in method that nests the dicts as @benjyw suggested. Thank you
@benjyw

benjyw approved these changes May 18, 2017

Looks good! Thanks.

@@ -429,14 +429,16 @@ def _create_dict_with_nested_keys_and_val(cls, keys, value):
:param primitive value: The value of the information being stored.
:return: dict of nested keys leading to the value.
"""
- current_index = len(keys) - 1
if len(keys) > 1:

This comment has been minimized.

@benjyw

benjyw May 18, 2017

Contributor

This looks really succinct and neat now. Sometimes Python has its moments...

@benjyw

benjyw May 18, 2017

Contributor

This looks really succinct and neat now. Sometimes Python has its moments...

@stuhood

This comment has been minimized.

Show comment
Hide comment
@stuhood

stuhood May 18, 2017

Member

Some breakage in CI, but looks network related. Going to merge optimistically.

Member

stuhood commented May 18, 2017

Some breakage in CI, but looks network related. Going to merge optimistically.

@stuhood stuhood merged commit a49a1b2 into pantsbuild:master May 18, 2017

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@stuhood stuhood added this to the 1.3.0 milestone May 18, 2017

stuhood added a commit that referenced this pull request May 18, 2017

Change method of reporting target data (#4593)
### Problem

In a previous version we were reporting target information by providing a string that is a key and a value which was a dict containing a lot of information. We want to be able to provide a list of keys that will recursively find the right place in a nested dict to store a primitive value (rather than a dictionary) to the reporting method.

### Solution

I created two helper methods to do this work. One recursively nests the keys with the value, and another does the work of finding the correct place in an existing dictionary to update with the new nested dict. 

### Result

These changes will affect how developers call the report_target_info API. As far as I know, no one is using it yet, so we don't need to make any changes to the dev documentation (apart from the method docs itself).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment