Python SR/MR cluster management#135
Conversation
trstephen-amazon
left a comment
trstephen-amazon
left a comment
There was a problem hiding this comment.
Overall LGTM but I guess CI found some issues with dict lookups in f-strings (thank you CI!)
| Delete all clusters that are: | ||
| 1. Not already deleting; and, | ||
| 2. Tagged with 'Repo=aws-samples/aurora-dsql-samples'; and, | ||
| 3. Tagged with 'Name=Python*' |
There was a problem hiding this comment.
not blocking: could even be a bit stricter here and check Python-CM-Example-*
|
aha, issue with the original commit was the f-string escapes are only supported in 3.12 (docs, h/t @danielfrankcom ) Our CI is using 3.10 and the readme wants Python >= 3.8 so we need to rationalize the code, readme, and CI on a min version. |
|
A possible way to determine a sensible Python version would be to figure out the minimum version supported by all Python drivers which we have samples for. I know Psycopg3 had a clear list of supported/tested Python versions when I was working on that sample. Using a single Python version across all Python samples in the project would help us keep track of these kinds of issues. |
| if cluster['status'] in ['DELETED', 'DELETING']: | ||
| continue | ||
|
|
||
| # Check tags to identify test clusters |
There was a problem hiding this comment.
not blocking: the updated SDK should already have tags in the GetCluster response
https://docs.aws.amazon.com/aurora-dsql/latest/APIReference/API_GetCluster.html
The latest version of Psycopg3 requires minimum 3.10, I don't remember any discussion about this but I'm guessing it's not a coincidence that we chose 3.10 in the CI. Psycopg2 only requires 3.8 though. Our READMEs should be double-checked to verify they match this. |
3 participants
By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT-0 license.
Thank you for your contribution!