Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(scf): [116337119] return functionId #2590

Merged
merged 3 commits into from
Apr 13, 2024
Merged

fix(scf): [116337119] return functionId #2590

merged 3 commits into from
Apr 13, 2024

Conversation

SevenEarth
Copy link
Collaborator

No description provided.

@andrew-tx andrew-tx added the run check run check label Apr 13, 2024
@@ -147,15 +148,21 @@ func (me *ScfService) CreateFunction(ctx context.Context, info scfFunctionInfo)
return nil
}

func (me *ScfService) DescribeFunction(ctx context.Context, name, namespace string) (resp *scf.GetFunctionResponse, err error) {
func (me *ScfService) DescribeFunction(ctx context.Context, name, namespace string, functionId ...string) (resp *scf.GetFunctionResponse, err error) {
Copy link
Collaborator

@andrew-tx andrew-tx Apr 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DescribeFunction 这个函数除了在 query 里面调用外,在其他地方有调用吗,包括其他的资源中有调用这个函数吗
函数的定义改成
DescribeFunction(ctx context.Context, name, namespace string, functionId string)
成本高吗

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果可以的话,下面这段可以简化吧:
image
保持跟之前一样就行,只需要
var iacExtInfo connectivity.IacExtInfo
iacExtInfo.InstanceId = strings.Join([]string{name, namespace}, tccommon.FILED_SP)
if err := resource.Retry(tccommon.ReadRetryTimeout, func() *resource.RetryError {
ratelimit.Check(request.GetAction())
response, err := me.client.UseScfClient(iacExtInfo).GetFunction(request)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DescribeFunction函数当前resource自己本身就有4个地方调用,所以这里不能将functionId作为参数必传,因为这是一个选填参数,强制增加functionId函数会影响其余地方的入参规则

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里无法保持一行哈,因为有不定长参数,需要判断是否传functionId

@@ -147,15 +148,21 @@ func (me *ScfService) CreateFunction(ctx context.Context, info scfFunctionInfo)
return nil
}

func (me *ScfService) DescribeFunction(ctx context.Context, name, namespace string) (resp *scf.GetFunctionResponse, err error) {
func (me *ScfService) DescribeFunction(ctx context.Context, name, namespace string, functionId ...string) (resp *scf.GetFunctionResponse, err error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果可以的话,下面这段可以简化吧:
image
保持跟之前一样就行,只需要
var iacExtInfo connectivity.IacExtInfo
iacExtInfo.InstanceId = strings.Join([]string{name, namespace}, tccommon.FILED_SP)
if err := resource.Retry(tccommon.ReadRetryTimeout, func() *resource.RetryError {
ratelimit.Check(request.GetAction())
response, err := me.client.UseScfClient(iacExtInfo).GetFunction(request)

@@ -868,7 +879,8 @@ func resourceTencentCloudScfFunctionRead(d *schema.ResourceData, m interface{})
}
namespace, name := split[0], split[1]

response, err := service.DescribeFunction(ctx, name, namespace)
functionId := d.Get("function_id").(string)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

acctest 测试用例增加下 functionid 的验证吧

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已补充

@andrew-tx andrew-tx added run check run check and removed run check run check labels Apr 13, 2024
Copy link
Collaborator

@andrew-tx andrew-tx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SevenEarth SevenEarth added run check run check and removed run check run check labels Apr 13, 2024
@SevenEarth
Copy link
Collaborator Author

本地测试结果:
创建
image
销毁
image
导入
image

@andrew-tx andrew-tx merged commit 6ca7637 into master Apr 13, 2024
10 checks passed
@andrew-tx andrew-tx deleted the fix/scf branch April 13, 2024 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run check run check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants