feat: New file destination plugin#6096
Conversation
| // This is used for debugging purposes only | ||
| NoRotate bool `json:"no_rotate,omitempty"` |
There was a problem hiding this comment.
Should it be removed, if it's only for debugging? What does it do?
There was a problem hiding this comment.
It is also for user usage. Added documentation under website
There was a problem hiding this comment.
We should update the comment above the line then, it currently says that it's for debugging purposes only
Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
erezrokah
left a comment
There was a problem hiding this comment.
Looks good, still need to test it locally.
Also since we have only 2 file types maybe do it as a package inside the plugin, not sure, to avoid the monorepo complexities, and only move it outside when it's used somewhere else than this plugin.
For example we don't have CI that runs the tests/linting/etc. on this module, so we'll need to create a separate one (or do it inside the file plugin CI)
plugins/destination/file/Makefile
Outdated
|
|
||
| .PHONY: lint | ||
| lint: | ||
| @golangci-lint run --config ../../.golangci.yml --timeout 10m --verbose |
There was a problem hiding this comment.
We can remove the timeout and verbose flags (we have the timeout in the config file, and verbose generates too much information for it to be useful)
There was a problem hiding this comment.
Looks good, still need to test it locally. Also since we have only 2 file types maybe do it as a package inside the plugin, not sure, to avoid the monorepo complexities, and only move it outside when it's used somewhere else than this plugin.
For example we don't have CI that runs the tests/linting/etc. on this module, so we'll need to create a separate one (or do it inside the file plugin CI)
Im going to open a PR for s3, gcs and azure in a moment after this one is merged so we need it to be a package
Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
hermanschaaf
left a comment
There was a problem hiding this comment.
LGTM apart from the linters failing right now
disq
left a comment
There was a problem hiding this comment.
The filename format could use a hash of all columns in table order (esp. in CSV) but maybe later I guess
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🤖 I have created a release *beep* *boop* --- ## 1.0.0 (2022-12-30) ### Features * New file destination plugin ([#6096](#6096)) ([748cca7](748cca7)) ### Bug Fixes * **file:** Use external filetypes library ([#6119](#6119)) ([a7219fe](a7219fe)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Docs and tests passing.