Skip to content

chore(pg): Add README#2042

Merged
yevgenypats merged 5 commits intomainfrom
chore/pg/docs
Sep 27, 2022
Merged

chore(pg): Add README#2042
yevgenypats merged 5 commits intomainfrom
chore/pg/docs

Conversation

@yevgenypats
Copy link
Copy Markdown
Contributor

Add general and config docs for PostgreSQL destination.

@cq-bot cq-bot added the cli label Sep 24, 2022
Copy link
Copy Markdown
Member

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

Added a few comments, I think the top level and spec comments could be relevant to other plugins


Known supported databases:

- PostgreSQL > v10
Copy link
Copy Markdown
Member

@erezrokah erezrokah Sep 25, 2022

Choose a reason for hiding this comment

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

Suggested change
- PostgreSQL > v10
- PostgreSQL with a version greater or equal to 11

Do we mean >= 11? Or v10.0.1 is supported too?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think v10.0.1 should be supported as well. at least per my quick check locally. I didn't use any new shiny primitives.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the only reason we supported 10 is because of Aurora which now supports 11 cc @bernays


## PostgreSQL Spec

This is the top level spec used by PostgreSQL Destination Plugin.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When we write top level, what does it mean?

To me it means that the yaml file looks like this:

kind: destination
connection_string: <>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we can maybe just omit the top level or even better just remove this line and go straight to the spec as it's a bit redundant. The user should be aware of our convention and that every plugin has a spec (it is mentioned in our main docs so no need to repeat that).

In any case it means the following:

kind: destination
spec:
  name: postgresql
  mode: overwrite
  spec:
     // goes here
     connection_string: stuff..

Similar to here - https://kubernetes.io/docs/reference/kubernetes-api/service-resources/service-v1/ there is no intro wording that this is a spec and we already have an intro about what is the postgresql destination plugin


- PostgreSQL > v10

## PostgreSQL Spec
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
## PostgreSQL Spec
## Configuration

Can we use configuration instead of spec?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we need to be consistent with the configuration as we use spec (we went the k8s way so should be familiar to some users) so would rather leave as is.

yevgenypats and others added 3 commits September 25, 2022 10:26
Co-authored-by: Erez Rokah <erezrokah@users.noreply.github.com>
Co-authored-by: Erez Rokah <erezrokah@users.noreply.github.com>
Co-authored-by: Erez Rokah <erezrokah@users.noreply.github.com>
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.

3 participants