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

Add acceptance tests for S3 #62

Open
maha-hajja opened this issue Jun 15, 2022 · 1 comment
Open

Add acceptance tests for S3 #62

maha-hajja opened this issue Jun 15, 2022 · 1 comment
Labels
feature New feature or request

Comments

@maha-hajja
Copy link
Contributor

maha-hajja commented Jun 15, 2022

Feature description

We already had a PR for this but it had many problems {all these are just things specific for acceptance tests to work on S3, they are not S3 related bugs}:
#44

  • The acceptance tests expect that the destination writes a record, and the source will retrieve the same record containing the same key and payload.. but this is not the case here because the key would be a random number.. and the payload would be the whole record {S3 destination has a batch size.. so the file name at the destination bucket can't have the same key.. so it's a random number.json}

  • the createdAt timestamp in the record is an int timestamp: ex: 1655131862580631000, and when we try to parse the file body into sdk.Record we get this error:

"CreatedAt":1655131862580631000
err: parsing time "1655131862580631000" as "\"2006-01-02T15:04:05Z07:00\"": cannot parse "1655131862580631000" as "\""
  • if the payload has any special characters, a back dash will be added to the string, which will make it not match, ex:
want: "Payload":{"'8B2\u0026U
got : "Payload":"{\"'8B2\\u0026U
  • the isEqualRecords method in acceptance tests assumes that payloads are unique, but in our case, the payload is the whole record.
@maha-hajja maha-hajja added the feature New feature or request label Jun 15, 2022
@meroxa-machine meroxa-machine moved this to Triage in Conduit Main Jun 15, 2022
@neovintage neovintage removed the status in Conduit Main Jul 11, 2022
@neovintage neovintage moved this to Triage in Conduit Main Jul 11, 2022
@neovintage neovintage removed the status in Conduit Main Jul 18, 2022
@neovintage
Copy link
Contributor

After we get through the performance benchmarks and opencdc we'll come back to revisit this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
Status: No status
Development

No branches or pull requests

2 participants