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

Docs for built-in processors #30

Merged
merged 21 commits into from
Apr 11, 2023
Merged

Docs for built-in processors #30

merged 21 commits into from
Apr 11, 2023

Conversation

hariso
Copy link
Contributor

@hariso hariso commented Apr 8, 2023

Fixes ConduitIO/conduit#966.

I've used the comments in the procbuiltin a great deal. The docs in Conduit mention raw keys/payloads with schema. While a concept of schema exists in Conduit, it's nowhere used. Because of that, I've removed the mention of "schema" here.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Apr 8, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4b7a9eb
Status: ✅  Deploy successful!
Preview URL: https://fc7ab826.conduit-site.pages.dev
Branch Preview URL: https://haris-processors.conduit-site.pages.dev

View logs

docs/processors/builtin.mdx Outdated Show resolved Hide resolved
Comment on lines 18 to 20
- If the key is raw and has a schema attached, extracts the field and uses it to replace the entire key.
- If the key is raw and has no schema, returns an error (not supported).
- If the key is structured, extracts the field and uses it to replace the entire key.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hariso hariso marked this pull request as ready for review April 9, 2023 15:22
@hariso hariso requested review from a team and simonl2002 April 9, 2023 15:22
Copy link
Member

@lovromazgon lovromazgon left a comment

Choose a reason for hiding this comment

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

I think it's a great start, in the future it would be great to also have examples for each processor. I think that is a bigger piece of work though and should be done separately.

docs/processors/builtin.mdx Outdated Show resolved Hide resolved
docs/processors/getting-started.mdx Outdated Show resolved Hide resolved
docs/processors/getting-started.mdx Show resolved Hide resolved
@hariso hariso requested a review from lovromazgon April 11, 2023 13:11
@hariso hariso requested a review from maha-hajja April 11, 2023 13:11
@hariso
Copy link
Contributor Author

hariso commented Apr 11, 2023

I think it's a great start, in the future it would be great to also have examples for each processor. I think that is a bigger piece of work though and should be done separately.

Yup, examples came to my mind as well, but having some good examples would take some time.


### parsejsonkey

Transforms the record's key into structured data, i.e. a JSON object.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this processor and parsejsonpayload need more details, maybe something like:

Suggested change
Transforms the record's key into structured data, i.e. a JSON object.
Transforms the record's raw data key (has to be JSON formatted) into structured data, i.e. a JSON object.

Copy link
Contributor

Choose a reason for hiding this comment

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

@hariso looks like you didn't notice the comment, we need to update the payload processor too 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, sorry about that! I'll get your changes in a separate branch. Thanks for calling this out!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maha-hajja Thinking again about this... IIUC, the parsejson pair of processors will parse the JSON even when the data is structured, which means that we don't need to say "transform the record's raw data key/payload.after...". Is my understanding correct? I agree that the note about that it has to be JSON formatted should be there.

Copy link
Contributor

Choose a reason for hiding this comment

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

@hariso if the data is already structured then it's just returned as is

Copy link
Member

@lovromazgon lovromazgon left a comment

Choose a reason for hiding this comment

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

🏅

@hariso hariso merged commit 480daf7 into main Apr 11, 2023
@hariso hariso deleted the haris/processors branch April 11, 2023 19:18

### parsejsonpayload

Transforms the record's `Payload.After` into structured data, i.e. a JSON object.
Copy link
Contributor

Choose a reason for hiding this comment

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

forgot to update payload processor:

Suggested change
Transforms the record's `Payload.After` into structured data, i.e. a JSON object.
Transforms the record's raw data `Payload.After` (has to be JSON formatted) into structured data, i.e. a JSON object.


### parsejsonkey

Transforms the record's key into structured data, i.e. a JSON object.
Copy link
Contributor

Choose a reason for hiding this comment

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

@hariso looks like you didn't notice the comment, we need to update the payload processor too 👀

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.

document built-in processors
3 participants