Updated DeployResourceCommand to support TenantId.

[ShawnAbshire]

Updates DeployResourceCommand to support TenantId.

This is one of two interfaces I would like to introduce for TenantId.

This begins the work to update the various builders to support multi-tenancy in 591

Splitting out into smaller PR's as requested by author.

[CLAassistant]

All committers have signed the CLA.

[ShawnAbshire]

@Zelldon This introduces a new interface I would like to apply to the other commands. If you agree with this approach and the PR is merged, I'll rebase and continue on updating the remaining interfaces.

this is one of two interfaces I want to introduce that deals with tenant ids. The second one can be seen in the other PR

[ChrisKujawa]

Thanks for your time and work @ShawnAbshire ! I really appreciate this!

I have one remark regarding the interface see below.

I follow https://devblogs.microsoft.com/appcenter/how-the-visual-studio-mobile-center-team-does-code-review/ for the code review JFYI.

Lets remove the interface and add the method to the end interface than we can go ahead.

[ChrisKujawa]

❌ Sorry, but I think this will break compatibility. I know it is more convenient especially to reuse the interface, but changing the order of interfaces and types will break backward compatibility. Lets not do this.

There is an Interface *Step3, which is left for optional parameters here you can add the method.

[ShawnAbshire]

Sorry, I'm not seeing an interface for the DeployResourceCommand for *Step3.

The only one I see is the IDeployResourceCommandBuilderStep2 which has a comment for optional parameters so I assume you're talking about that one and I've updated the PR to remove the new interface and added the new method there.

I am curious on how adding a new optional parameter to the IDeployResourceCommandStep1 would break compatibility. Feels weird specifying the tenantId at the end of the builder as opposed to early on. I'm not pushing for either but this does effect updating all the other commands on where to place the new option. At the end or at the beginning.

[ChrisKujawa]

Sorry for the late reply I was busy with the normal Zeebe project work.

The only one I see is the IDeployResourceCommandBuilderStep2 which has a comment for optional parameters
Yeah sorry I meant this one.

I am curious on how adding a new optional parameter to the IDeployResourceCommandStep1 would break compatibility. Feels weird specifying the tenantId at the end of the builder as opposed to early on. I'm not pushing for either but this does effect updating all the other commands on where to place the new option. At the end or at the beginning.

Imagine you have already code running/deployed like

 var deployResponse = await client.NewDeployCommand()
                .AddResourceFile(DemoProcessPath)
                .Send();

or even

 var deployResponse = await client.NewDeployCommand()
                .AddResourceFile(DemoProcessPath)
                .AddResourceFile(DemoProcessPath2)
                .Send();

If you add a tenantId in between or in front you will break this code. Of course it wouldn't break if you made it optional as well in the interface, but changing the type of the build could potential also break others code, since people something keep the returned builders.

// if not using var this is an issue (for example when storing it in a field)
  IDeployResourceCommandStep2 base = await client.NewDeployCommand()
                .AddResourceFile(DemoProcessPath)
                .AddResourceFile(DemoProcessPath2)
// later
base.AddResourceFile(DemoProcessPath)

[ChrisKujawa]

I just checked how it is done in the java client and it was also added at the end, btw similar how you did with an interface but on a different place https://github.com/camunda/zeebe/blob/main/clients/java/src/main/java/io/camunda/zeebe/client/api/command/DeployResourceCommandStep1.java#L100

[ShawnAbshire]

Sounds good. The code has been updated to remove the interface. Let me know if you want to move forward with updating the other commands or if you'd prefer to move back to using the interface and apply before the final steps.

[ChrisKujawa]

Yes lets recreate the interface and to it always on final stage. Sorry for the back and forth.

[ShawnAbshire]

Done.

[ChrisKujawa]

One more thing could you take a look at the commit guidelines https://github.com/camunda/zeebe/blob/main/CONTRIBUTING.md#commit-message-guidelines

try to use a type prefix for your commit messages, like feat: when adding a feature of refactor: when doing a refactoring.

[ShawnAbshire]

Updated commit messages to follow guidelines.

[ChrisKujawa]

Sorry for the delay! Quite a busy week 🙇🏼

Thanks for your contribution again and your patience. The changes look good!