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 docs on custom Operations #3507

Merged
merged 14 commits into from
Jun 14, 2021
Merged

Conversation

ChristopheDuong
Copy link
Contributor

@ChristopheDuong ChristopheDuong commented May 20, 2021

What

Close #3237

┆Issue is synchronized with this Asana task by Unito

@auto-assign auto-assign bot requested review from cgardens and davinchia May 20, 2021 17:00
Copy link
Contributor

@davinchia davinchia left a comment

Choose a reason for hiding this comment

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

Thanks for this @ChristopheDuong ! This is awesome.

Most of my comments are phrasing suggestions. Two medium things about better organising the operations page and figuring out a good place to place the steps on dbt with private repos.

I know I commented a bunch about UI snapshot - I realise this is blocking on Artem. Either removing those lines or waiting for him to merge is fine by me.

Not approving until we figure those out.

@@ -52,21 +59,34 @@ Connection:
keepalives_idle: 0
sslmode: None
Connection test: OK connection ok
```
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: small comment pointing out what to look for. it's a lot to parse otherwise.

my comment is less relevant if 'everything' being 'ok' is what we expect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, everything should be 'OK'

It's mainly to see the expected output of the dbt command look like too

select * from covid_epidemiology_with_id
]) }} as _airbyte_covid_epidemiology_hashid
from {{ ref('covid_epidemiology_ab2_558') }}
-- covid_epidemiology
Copy link
Contributor

Choose a reason for hiding this comment

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

love this example!

@@ -248,3 +248,6 @@ Note that all the choices made by Normalization as described in this documentati
* to build a [custom SQL view]() with your own naming conventions
* to export, edit and run [custom DBT normalization](https://github.com/airbytehq/airbyte/tree/e378d40236b6a34e1c1cb481c8952735ec687d88/docs/tutorials/transformation-and-normalization/transformations-with-dbt.md) yourself

## UI Configurations

Screenshots of examples of normalization settings here
Copy link
Contributor

Choose a reason for hiding this comment

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

is this WIP?

Copy link
Contributor

Choose a reason for hiding this comment

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

should we remove before merging in?


run --models tag:covid_api opendata.base.*

Screenshots of examples of normalization settings for my private git repo here
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment on making sure we do this before merging.

docs/understanding-airbyte/operations.md Outdated Show resolved Hide resolved
docs/understanding-airbyte/operations.md Outdated Show resolved Hide resolved
Co-authored-by: Davin Chia <davinchia@gmail.com>
@ChristopheDuong ChristopheDuong marked this pull request as draft May 21, 2021 16:38
@ChristopheDuong
Copy link
Contributor Author

ChristopheDuong commented May 21, 2021

I'm marking the PR it as draft while waiting for UI implementation to add the screenshots

Copy link
Contributor

@avaidyanatha avaidyanatha left a comment

Choose a reason for hiding this comment

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

This may seem like a lot of comments, but it's mostly just wording/grammar/clarity fixes - thank you so much @ChristopheDuong for writing this up :) ❤️

EDIT: Also I realize that this PR is blocked on a few other things as @davinchia mentioned. So take my approval as more of a check on wording/clarity rather than an OK to merge 😅

After replication of data from a source connector (Extract) to a destination connector (Load), multiple optional transformations steps can now be applied as part of an Airbyte Sync. Possible transformations are:

1. Basic normalization transformations as automatically generated by Airbyte dbt code generator.
2. Customized normalization transformations as edited by the user (and disable the default generated normalization one)
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
2. Customized normalization transformations as edited by the user (and disable the default generated normalization one)
2. Customized normalization transformations as edited by the user (which disables the default generated normalization one)

docs/understanding-airbyte/operations.md Outdated Show resolved Hide resolved
docs/understanding-airbyte/operations.md Outdated Show resolved Hide resolved
docs/understanding-airbyte/operations.md Outdated Show resolved Hide resolved
docs/understanding-airbyte/operations.md Outdated Show resolved Hide resolved
docs/understanding-airbyte/operations.md Outdated Show resolved Hide resolved
ChristopheDuong and others added 3 commits May 25, 2021 11:26
Copy link
Contributor

@davinchia davinchia left a comment

Choose a reason for hiding this comment

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

Feel free to merge whenever you add all the pictures!

Comment on lines +28 to +31
* [Full Refresh Overwrite](full-refresh-overwrite.md): Sync the whole stream and replace data in destination by overwriting it.
* [Full Refresh Append](full-refresh-append.md): Sync the whole stream and append data in destination.
* [Incremental Append](incremental-append.md): Sync new records from stream and append data in destination.
* [Incremental Deduped History](incremental-deduped-history.md): Sync new records from stream and append data in destination, also provides a de-duplicated view mirroring the state of the stream in the source.
Copy link
Member

Choose a reason for hiding this comment

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

@ChristopheDuong can add a note saying that for the first run Incremental mode works as a full refresh ?

@ChristopheDuong ChristopheDuong changed the base branch from master to chris/namespace-docs June 4, 2021 12:39
@ChristopheDuong ChristopheDuong changed the base branch from chris/namespace-docs to master June 4, 2021 12:40
@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Jun 11, 2021
@ChristopheDuong ChristopheDuong marked this pull request as ready for review June 14, 2021 08:31
@ChristopheDuong ChristopheDuong merged commit 22b75c5 into master Jun 14, 2021
@ChristopheDuong ChristopheDuong deleted the chris/custom-dbt-docs branch June 14, 2021 08:31
@ChristopheDuong ChristopheDuong changed the title Add docs on custom Operations 🎉 Add docs on custom Operations Jun 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom Transformations: Document new feature
4 participants