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 custom files API support #368

Merged
merged 1 commit into from
Dec 19, 2023
Merged

Conversation

van-ess0
Copy link
Contributor

This commit adds custom file support for Aiven services

@van-ess0 van-ess0 requested review from a team as code owners October 25, 2023 16:24
@van-ess0 van-ess0 requested review from ilpon and mteiste October 25, 2023 16:24
@van-ess0 van-ess0 force-pushed the van_ess0_custom_files_api_support branch from 2b19d82 to d8a6bd9 Compare October 25, 2023 16:46
aiven/client/cli.py Outdated Show resolved Hide resolved
aiven/client/cli.py Outdated Show resolved Hide resolved
aiven/client/cli.py Show resolved Hide resolved
@van-ess0 van-ess0 force-pushed the van_ess0_custom_files_api_support branch from d8a6bd9 to 6a4fa10 Compare October 26, 2023 07:07
@van-ess0 van-ess0 dismissed mteiste’s stale review October 26, 2023 07:08

Fixed docstrings

@van-ess0 van-ess0 requested review from mteiste and Prime541 October 26, 2023 07:08
@van-ess0 van-ess0 force-pushed the van_ess0_custom_files_api_support branch 2 times, most recently from a08839b to 60ad2bf Compare October 26, 2023 07:40
heikju
heikju previously requested changes Nov 17, 2023
aiven/client/client.py Show resolved Hide resolved
aiven/client/client.py Outdated Show resolved Hide resolved
aiven/client/cli.py Outdated Show resolved Hide resolved
@heikju
Copy link
Contributor

heikju commented Nov 17, 2023

It would also be nice to see tests for this. The test coverage for client is not great, but at least we would have an opportunity here to make it slightly better.

@van-ess0 van-ess0 force-pushed the van_ess0_custom_files_api_support branch from 60ad2bf to 6647a30 Compare November 21, 2023 17:06
@van-ess0 van-ess0 requested review from rikonen and heikju November 21, 2023 17:08
@van-ess0 van-ess0 force-pushed the van_ess0_custom_files_api_support branch from 6647a30 to 01b8787 Compare November 21, 2023 17:10
@van-ess0 van-ess0 dismissed heikju’s stale review November 21, 2023 17:11

Fixed specified issues

@van-ess0 van-ess0 force-pushed the van_ess0_custom_files_api_support branch 5 times, most recently from b3091a5 to 028e00a Compare November 21, 2023 17:49
Add custom files CRUD support
Reduce client.verify complexity
@van-ess0 van-ess0 force-pushed the van_ess0_custom_files_api_support branch from 028e00a to 15c4ad1 Compare November 21, 2023 17:53
@ilpon ilpon removed their request for review December 11, 2023 05:49
@arg.service_name
@arg("--file_id", help="A file ID, usually an uuid4 string", required=True)
@arg("--target_filepath", help="Where to save the file downloaded")
@arg("--stdout_write", help="Write to the stdout instead of file", action="store_true")
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this flag makes sense. E.g from the face of it (if I read this correctly) the default if target_filepath is not given is to write to stdout, so the question is what is the use case for getting the file but not writing it to anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see it like fetch file | grep foo | sed 's/o/i/' >> new_file.txt

@mteiste mteiste merged commit dce793a into main Dec 19, 2023
28 checks passed
@mteiste mteiste deleted the van_ess0_custom_files_api_support branch December 19, 2023 08:48
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