add blueray cluster support for saving the cluster information #63
Conversation
| } | ||
|
|
||
| // 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?
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.
|
|
||
| func (p *PulumiMySQLRunRecorder) findBluerayStackFromClusterFQDN(ctx context.Context, clusterFQDN string) *PulumiResource { | ||
| // Extract cluster name as the first part of the FQDN (before the first dot) | ||
| parts := regexp.MustCompile(`\.`).Split(clusterFQDN, 2) |
There was a problem hiding this comment.
Here you do not need to use regex because you just need to get the part before the dot.
| parts := regexp.MustCompile(`\.`).Split(clusterFQDN, 2) | |
| parts := strings.SplitN(clusterFQDN, ".", 2) |
|
|
||
| func (p *PulumiMySQLRunRecorder) findStackFromClusterFQDN(ctx context.Context, clusterFQDN string) *PulumiResource { | ||
| // Check if the cluster FQDN matches the Presto DB pattern | ||
| if regexp.MustCompile(`.+\.ibm\.prestodb\.dev`).MatchString(clusterFQDN) { |
There was a problem hiding this comment.
| if regexp.MustCompile(`.+\.ibm\.prestodb\.dev`).MatchString(clusterFQDN) { | |
| if strings.HasSuffix(clusterFQDN, ".ibm.prestodb.dev") { |
| } | ||
|
|
||
| // Check if the cluster FQDN matches the blueray pattern | ||
| if regexp.MustCompile(`.+\.cloud\.ibm\.com`).MatchString(clusterFQDN) { |
There was a problem hiding this comment.
| if regexp.MustCompile(`.+\.cloud\.ibm\.com`).MatchString(clusterFQDN) { | |
| if strings.HasSuffix(clusterFQDN, ".cloud.ibm.com") { |
| 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.
| expectedClusterName := "xlarge-b109n-yabin-eng" | ||
|
|
||
| // Call the function | ||
| ctx := context.Background() |
There was a problem hiding this comment.
context can be removed from findBluerayStackFromClusterFQDN
| 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)| } | ||
|
|
||
| // 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)| } | ||
|
|
||
| // 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())3 participants
No description provided.