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

doc(fivetran_sdk): Updated development guide for partners #43

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

fivetran-satvikpatil
Copy link
Contributor

Closes https://fivetran.height.app/T-711151

Description of Changes

Updated development guide for partners

@fivetran-satvikpatil fivetran-satvikpatil self-assigned this Jun 7, 2024
@fivetran-satvikpatil fivetran-satvikpatil marked this pull request as ready for review June 7, 2024 09:48
Copy link
Collaborator

@manjutapali manjutapali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to reformat the statements.

development-guide.md Outdated Show resolved Hide resolved
development-guide.md Outdated Show resolved Hide resolved
development-guide.md Outdated Show resolved Hide resolved
development-guide.md Outdated Show resolved Hide resolved
development-guide.md Outdated Show resolved Hide resolved
Copy link
Contributor

@5tran-alexil 5tran-alexil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my suggestions

development-guide.md Outdated Show resolved Hide resolved
development-guide.md Outdated Show resolved Hide resolved
development-guide.md Outdated Show resolved Hide resolved
development-guide.md Outdated Show resolved Hide resolved
development-guide.md Outdated Show resolved Hide resolved
Copy link
Contributor

@5tran-alexil 5tran-alexil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

development-guide.md Outdated Show resolved Hide resolved
development-guide.md Outdated Show resolved Hide resolved
development-guide.md Outdated Show resolved Hide resolved
development-guide.md Outdated Show resolved Hide resolved
development-guide.md Outdated Show resolved Hide resolved
development-guide.md Outdated Show resolved Hide resolved
@fivetran-satvikpatil fivetran-satvikpatil changed the base branch from main to ft-sdk-intf-changes June 20, 2024 06:23
Base automatically changed from ft-sdk-intf-changes to main June 21, 2024 12:09
Copy link
Contributor

@fivetran-abdulsalam fivetran-abdulsalam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have requested some changes


- `update_files` is for `update` operation where modified columns have actual values whereas unmodified columns have the special value `unmodified_string` in `CsvFileParams`. Soft-deleted rows will arrive in here as well. Update the `_fivetran_synced` column in the destination with the values coming in from the csv files.
- `update_files` is for the `update` operation where modified columns have actual values whereas unmodified columns have the special value `unmodified_string` in `FileParams`. Soft-deleted rows will arrive in here as well. Update the `_fivetran_synced` column in the destination with the values coming in from the batch files.
Copy link
Contributor Author

@fivetran-satvikpatil fivetran-satvikpatil Jun 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I will merging the CsvFileParams and ParquetFileParams into one i.e. FileParams.
Ref: https://fivetran.height.app/T-725623

- `keys` is a map that provides a list of secret keys, one for each batch file, that can be used to decrypt them.

- `file_params` provides information about the file type and any configurations applied to it, such as encryption or compression.

Also, Fivetran will deduplicate operations such that each primary key will show up only once in any of the operations

Do not assume order of columns in the batch files. Always read the CSV file header to determine column order.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Do not assume order of columns in the batch files. Always read the CSV file header to determine column order.
Do not assume order of columns in the batch files. Always read the columns of batch file to determine column order.

Copy link
Contributor

@fivetran-abdulsalam fivetran-abdulsalam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with a nit comment

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

Successfully merging this pull request may close these issues.

5 participants