Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SequenceUploadQueue add_to_upload_queue incorrectly describes column_external_ids #212

Open
sceniclife opened this issue May 11, 2023 · 0 comments

Comments

@sceniclife
Copy link

def add_to_upload_queue(
self,
rows: Union[
Dict[int, List[Union[int, float, str]]],
List[Tuple[int, Union[int, float, str]]],
List[Dict[str, Any]],
SequenceData,
],
column_external_ids: Optional[List[dict]] = None,
id: int = None,
external_id: str = None,
) -> None:
"""
Add sequence rows to upload queue. Mirrors implementation of SequenceApi.insert. Inserted rows will be
cached until uploaded
Args:
rows: The rows to be inserted. Can either be a list of tuples, a list of ["rownumber": ..., "values": ...]
objects, a dictionary of rowNumber: data, or a SequenceData object.
column_external_ids: List of external id for the columns of the sequence
id: Sequence internal ID
Use if external_id is None
external_id: Sequence external ID
Us if id is None
"""

The parameter description states:
column_external_ids: List of external id for the columns of the sequence

But the parameter takes in:
column_external_ids: Optional[List[dict]] = None

As opposed to a List[str]. It then will try to plug it into the columns parameter SequenceData(id=id, external_id=id, rows=rows, columns=column_external_ids) which will later result in error:

return [cast(str, c.get("externalId")) for c in self.columns]

SequenceData is expecting an API like dict for columns:
https://docs.cognite.com/api/v1/#tag/Sequences/operation/createSequence

Possible options:

  1. Change the parameter for add_to_upload_queue from column_external_ids to columns but that will change the look and feel. The SDK feel is to insert a list of str as column_external_ids.
  2. Keep it as column_external_ids and in add_to_upload_queue, generate the API like dict for columns from the list of str column_external_ids.
  3. Within the SequenceUploadQueue, there is a function set_sequence_column_definition that sets self.column_definitions but it isn't used unless a create is being called. It feels super odd when you want to create on missing Sequences and call set_sequence_column_definition with column definitions, but when calling add_to_upload_queue, you must additionally add column definitions again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant