fix: gcs_object_metadata_client can't handle required_tasks

[kitagry]

fix #466

This pull request fix the _get_patched_obj_metadata method in gokart/gcs_obj_metadata_client.py.
In original code, we doesn't consider str type, so RecursionError was occurred. like #466 .

In this PR, I made the code simple and add tests in order to check the behaviors.

[hiro-o918]

LGTM!

[kitagry]

@TlexCypher Hi! Could you review this?

[Copilot]

Pull Request Overview

This PR adds support for serializing required_task_outputs into object metadata in GCSObjectMetadataClient and includes a new test for this functionality.

• Extend _get_patched_obj_metadata to include required_task_outputs
• Implement _get_serialized_string to handle single, dict, and iterable inputs with type checking
• Add a unit test for the new required_task_outputs parameter

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
test/test_gcs_obj_metadata_client.py	Imported RequiredTaskOutput and added test_get_patched_obj_metadata_with_required_task_outputs
gokart/gcs_obj_metadata_client.py	Removed early return, added support for required_task_outputs, updated _get_serialized_string logic

Comments suppressed due to low confidence (5)

gokart/gcs_obj_metadata_client.py:117

• The return type annotation FlattenableItems[str] doesn't reflect that this method may return dicts or lists; consider updating the type hint and/or renaming the method to match its actual outputs.

def _get_serialized_string(required_task_outputs: FlattenableItems[RequiredTaskOutput]) -> FlattenableItems[str]:

gokart/gcs_obj_metadata_client.py:98

• The new required_task_outputs parameter isn't described in this method's docstring; please update the docstring to explain its purpose and supported types.

def _get_patched_obj_metadata(

test/test_gcs_obj_metadata_client.py:117

• Add tests for passing a single RequiredTaskOutput instance and a dict of outputs to cover all branches in _get_serialized_string.

def test_get_patched_obj_metadata_with_required_task_outputs(self):

test/test_gcs_obj_metadata_client.py:117

• Include a test case that verifies a TypeError is raised when an unsupported type is passed to required_task_outputs.

def test_get_patched_obj_metadata_with_required_task_outputs(self):

gokart/gcs_obj_metadata_client.py:122

• Ensure Iterable is imported (from collections.abc or typing) to avoid a potential NameError at runtime.

elif isinstance(required_task_outputs, Iterable):

[Hi-king]

Ah—since str is also an Iterable, calling:

_iterable_flatten([["a"]]) will never terminate.

The following implementation of _iterable_flatten works correctly:

def _iterable_flatten(nested_list: Iterable) -> Iterable[str]:
    for item in nested_list:
        if isinstance(item, str):
            yield item
        elif isinstance(item, Iterable):
            yield from _iterable_flatten(item)
        else:
            yield item

However, since recursion is only needed for _get_serialized_string, you removed iterable_flatten entirely, right? @kitagry

[kitagry]

yes👍

[Hi-king]

@kitagry Please include that background in the PR comment for future reference.

[Hi-king]

Confirmed this logic clearly converts FlattenableItems[RequiredTaskOutput] into FlattenableItems[str]. :)

LGTM.

[tyzerrr]

@kitagry
Sorry to be late for replying...
Thank u for fix my bug..!