Conversation
|
Hi Istvans, I created this pull request so that I can review the example python script that you made. It looks good to me, I just have some stylistic things that I would change personally. Let me know what you think. |
| # Check S3 bucket | ||
| try: | ||
| response = s3_client.list_objects_v2(Bucket=S3_BUCKET_NAME, Prefix=prefix) | ||
| # for obj in response.get('Contents', []): |
There was a problem hiding this comment.
Can you remove the commented out lines of code? I think that as an example it needs to be as clean as possible.
| global dl_min | ||
| global s3_client | ||
| global cnt_ok | ||
| global cnt_fail |
There was a problem hiding this comment.
The global keyword typically considered bad practice, even though I do see why you used it here.
I would however, remove the entire delete_old_files logic and leave that up to the user. Again since this is supposed to be an example I would make it as concise as possible.
If you delete the entire delete_old_files logic, you would not need to have these global keywords here anymore, as you would not need to be keeping a state any longer.
| # Delete files older than dl_min minutes | ||
| dl_min = 2 | ||
| dl_count = 0 | ||
| dl_dir = os.getenv("ODIM_DL_DIR", "./odim_files") |
There was a problem hiding this comment.
I believe dl_dir is a constant value right. According to variable name conventions it would make sense to make the name all caps
No description provided.