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

ReadRecords seems to have wrong definition #17

Open
andforslow opened this issue Mar 30, 2023 · 2 comments
Open

ReadRecords seems to have wrong definition #17

andforslow opened this issue Mar 30, 2023 · 2 comments

Comments

@andforslow
Copy link

Expected Behavior

Please describe the behavior you are expecting

I expected ReadRecords to follow the python definition:

def read_records(
    self, stream_state: Mapping[str, Any] = None, stream_slice: Optional[Mapping[str, Any]] = None, **kwargs
) -> Iterable[Mapping[str, Any]]:

In C#, this would look something like:

public FluentStreamBuilder ReadRecords(
  Func<JsonElement, JsonElement, IEnumerable<JsonElement> func)
{
  ...
}

Note that this function returns an iterable JsonElement.

Current Behavior

What is the current behavior?

Currently, the ReadRecords function returns a Task variable. The function looks like this when I look at the source code:

/// <summary>
/// This method should be overridden by subclasses to read records based on the inputs
/// </summary>
/// <param name="func">Input: SyncMode syncmode, ChannelWriter streamchannel, long? recordlimit, Dictionary cursorfield, Dictionary streamslice, JsonElement streamstate | Output Task></param>
public FluentStreamBuilder ReadRecords(
  Func<AirbyteLogger, SyncMode, ChannelWriter<AirbyteMessage>, long?, string[],
      Dictionary<string, object>, Dictionary<string, object>, Task<long>> func) =>
  AddFunc(func, nameof(ReadRecords));

This is not in line with the python function, which makes it very hard to mimic the behavior of connectors implemented in python. As an example to look at (which I've used for the above python example) is the BigCommerce connector.

Moreover, the docstring for this function is weird. The parameter definitions in the param name section are incoherent with the actual parameters of the function.

Failure Information (for bugs)

Please help provide information about the failure if this is a bug. If it is not a bug, please remove the rest of this template.

Read the information above.

Steps to Reproduce

Please provide detailed steps for reproducing the issue.

Again, read the information above.

Context

Please provide any relevant information about your setup. This is important in case the issue is not reproducible except for under certain conditions.

I am using the latest version of Airbyte.CDK.DotNet (0.0.6).

Failure Logs

Please include any relevant log snippets or files here.

-

@andforslow
Copy link
Author

As a background to what I want to achieve, it's basically this:

Given an endpoint https://api.domain.com/base_data which returns a payload like this: [ { "id": 1, ... }, ...], I want to incrementally pull data from a second endpoint https://api.domain.com/child_data/{base_data_id}, which has a url parameter based on the base_data id. The BigCommerce example I showed above has relatively straightforward code to deal with this scenario, but I can't see any way of achieving this with the C# CDK. This is a dealbreaker if the functionality isn't possible to achieve. Should I just switch to the python CDK instead?

@mrhamburg
Copy link
Owner

Hi @andforslow, thank you for checking out this CDK. Do know that this CDK has not been updated for a while. Back then (and I am not aware of any changes in this), the python CDK did not implement a builder pattern for creating connectors. Next to this the C# implementation uses channels to allow parallel streams in the future, another feature the python CDK did not support (it was on the backlog somewhere but no idea what the latest status is).

Regarding your request, I understand that you want to create a dependency between multiple streams where the output of one is the input of another stream. When the C# SDK was created there was no native way to do this in the Python CDK nor was there any time to implement this in the C# version. If the Python CDK has been further developed to accommodate your use case, I would suggest looking into the Python CDK version as it is maintained by Airbyte and more up to date than the C# version.

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

2 participants