add blueray cluster support for saving the cluster information #63
Conversation
stage/pulumi_run_recorder.go
Outdated
| } | ||
|
|
||
| // Check if the cluster FQDN matches the blueray pattern | ||
| if regexp.MustCompile(`.+\.cloud\.ibm\.com`).MatchString(clusterFQDN) { |
There was a problem hiding this comment.
Do you know if we can use our internal fork?
ethanyzhang
left a comment
ethanyzhang
left a comment
There was a problem hiding this comment.
Hi @yabinma thanks for making this change. Two major pieces of feedback:
- In the new checks you added for BlueRay we do not need to use regular expressions.
- Recommended to use test libraries to simplify the test code.
stage/pulumi_run_recorder.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func (p *PulumiMySQLRunRecorder) findBluerayStackFromClusterFQDN(ctx context.Context, clusterFQDN string) *PulumiResource { |
There was a problem hiding this comment.
you actually do not need a context.Context here
|
|
||
| import ( | ||
| "context" | ||
| "testing" |
There was a problem hiding this comment.
You can import "github.com/stretchr/testify/assert" and "github.com/stretchr/testify/require" here and use assert/require libraries below. You can check client_test.go as an example.
stage/pulumi_run_recorder_test.go
Outdated
| expectedClusterName := "xlarge-b109n-yabin-eng" | ||
|
|
||
| // Call the function | ||
| ctx := context.Background() |
There was a problem hiding this comment.
context can be removed from findBluerayStackFromClusterFQDN
stage/pulumi_run_recorder_test.go
Outdated
| resource := recorder.findBluerayStackFromClusterFQDN(ctx, testFQDN) | ||
|
|
||
| // Verify resource is not nil | ||
| if resource == nil { |
There was a problem hiding this comment.
You can do
require.NotNil(t, resource)
stage/pulumi_run_recorder_test.go
Outdated
| } | ||
|
|
||
| // Verify the cluster FQDN | ||
| if resource.Outputs.ClusterFQDN != testFQDN { |
There was a problem hiding this comment.
You can do
assert.Equal(t, testFQDN, resource.Outputs.ClusterFQDN)
stage/pulumi_run_recorder_test.go
Outdated
| } | ||
|
|
||
| // Verify Created timestamp is not zero | ||
| if resource.Created.IsZero() { |
There was a problem hiding this comment.
you can do
assert.False(t, resource.Created.IsZero())There was a problem hiding this comment.
Thank you @ethanyzhang for the review. All the changes done and test passed. Please help review it again.
3 participants
No description provided.