Ruby SR/MR cluster management#142
Conversation
| source "https://rubygems.org" | ||
|
|
||
| gem "aws-sdk-dsql", "~> 1" | ||
| gem "aws-sdk-dsql", "1.8.0" |
There was a problem hiding this comment.
Can use "~> 1.8" to keep pulling in versions [1.8 .. 2)
|
|
||
| gem 'rspec', '3.13.0' | ||
| gem 'aws-sdk-core', '3.211.0' No newline at end of file | ||
| gem 'aws-sdk-core', '~> 3' No newline at end of file |
There was a problem hiding this comment.
Is this required? pretty sure I was able to run locally without it.
| * Ruby version >=3.3 is needed | ||
| - Ruby version >= 2.5 is installed. |
There was a problem hiding this comment.
Ah, 2.5 matches CI but that version has been EOL since 2021. Can you bump CI to 3.3?
| export REGION_2="us-east-2" | ||
| export WITNESS_REGION="us-west-2" | ||
|
|
||
| bundle install |
There was a problem hiding this comment.
not blocking, comment on existing behavior: This can be painful if it targets system ruby (MacOS). It's not guaranteed to fail but we can suggest folks set up rbenv before running. We do this in rails instructions.
| cluster_1, cluster_2 = create_multi_region_clusters(region_1, region_2, witness_region) | ||
| cluster_id_1 = cluster_1["identifier"] | ||
| cluster_id_2 = cluster_2["identifier"] | ||
| raise "Cluster_1 identifier should not be null" if cluster_id_1.nil? |
There was a problem hiding this comment.
Consider this:
it "goes through a full life cycle" do
cluster_1, cluster_2 = create_multi_region_clusters(...)
expect { cluster_1["status"] }.to eq "ACTIVE"
# and so on: update -> get -> delete
endThe idea is you hoist all your operations outside the expect block and transform your raise into expect
| delete_cluster(region_1, cluster_id) | ||
|
|
||
| }.not_to raise_error | ||
| end |
There was a problem hiding this comment.
Optional: you could add a cluster sweeper (java ex) to an after(:context) block [rspec docs].
There was a problem hiding this comment.
Might be wasted effort if it's not already implemented in Ruby - I should have a shared version up for PR tomorrow which can be reused here
There was a problem hiding this comment.
it's already written, just need to wire it up in a test :D
I am excited to see this reusable version 🥁
trstephen-amazon
left a comment
trstephen-amazon
left a comment
There was a problem hiding this comment.
A couple optional enhancements. 🚢
| ```bash | ||
| # Optional use rbenv | ||
| rbenv install 3.3.5 | ||
| rbenv local 3.3.5 |
There was a problem hiding this comment.
FYI: you can set a .rbenv file in the root directory with contents
3.3.5
to set automatically.
| describe 'perform multi-region smoke tests' do | ||
| it 'does not raise any exception' do |
There was a problem hiding this comment.
nit: describes old test behavior
danielfrankcom
left a comment
danielfrankcom
left a comment
There was a problem hiding this comment.
Only small thing is we should align the environment variable names with the other examples as discussed offline for consistency.
…amples into Ruby_CM_Update
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!