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

Java Client code example not working #1881

Merged
merged 5 commits into from
Aug 7, 2023
Merged

Java Client code example not working #1881

merged 5 commits into from
Aug 7, 2023

Conversation

jenshb
Copy link
Contributor

@jenshb jenshb commented Mar 31, 2023

Code example is not working because zeebeApi is not defined. By trial and error figured out, that the Zeebe Adress without Port has to be passed here.

What is the purpose of the change

Working code example

Are there related marketing activities

No

When should this change go live?

No

PR Checklist

  • My changes apply to an already released version, and I have added them to the relevant /versioned_docs directory, or they are not for an already released version.
  • My changes apply to future versions, and I have added them to the main /docs directory, or they are not for future versions.
  • My changes require an Engineering review, and I've assigned an engineering manager or tech lead as a reviewer, or my changes do not require an Engineering review.
  • My changes require a technical writer review, and I've assigned @christinaausley as a reviewer, or my changes do not require a technical writer review.

@akeller
Copy link
Member

akeller commented Mar 31, 2023

@berndruecker is this behavior correct? The address doesn't include the port?

@berndruecker
Copy link
Member

I think we should actually better include sample addresses to make clear what a user needs - this is more intuitive to understand.

See https://javadoc.io/doc/io.camunda/zeebe-client-java/latest/io/camunda/zeebe/client/ZeebeClientBuilder.html#gatewayAddress(java.lang.String) (and https://github.com/camunda-community-hub/spring-zeebe#configuring-camunda-platform-8-saas-connection) - the gateway address includes the port (!)

We could probably just use 127.0.0.1:26500 as example to make this clear?

WDYT @jenshb?

@jenshb
Copy link
Contributor Author

jenshb commented Apr 3, 2023

Hi @berndruecker,

I also thought about providing examples, an example says more than thousand words ;-)
In the code example, the zeebeAPI is written into audience and gatewayAddress.
When I set the zeebeAPI to the ZEEBE_ADDRESS provided by Operate on creating a new client (in my case
ZEEBE_ADDRESS='0f6517db-6973-4604-af18-4f7f2583042a.dsm-1.zeebe.camunda.io:443')
I get an HTTP return code 401.

As far as I can see, this is because the audience should not contain the port but has to be
"0f6517db-6973-4604-af18-4f7f2583042a.dsm-1.zeebe.camunda.io". gatewayAddress works with "0f6517db-6973-4604-af18-4f7f2583042a.dsm-1.zeebe.camunda.io:443"

@christinaausley christinaausley added the component:docs Documentation improvements, including new or updated content label Apr 8, 2023
@christinaausley
Copy link
Contributor

My renaming work with APIs/clients will likely cause conflicts here. Once you've completed the change, please loop me in if you need assistance with a clean build here.

@berndruecker
Copy link
Member

Not sure if @jenshb is still working on this? I think we can merge the current proposal, still I personally would prefer an example - not the rule "gateway address without port" - but it is an improvement anyway

Code example is not working because zeebeApi is not defined. By trial and error figured out, that the Zeebe Adress without Port has to be passed here.
@CLAassistant
Copy link

CLAassistant commented Apr 24, 2023

CLA assistant check
All committers have signed the CLA.

@jenshb
Copy link
Contributor Author

jenshb commented Apr 24, 2023

I introduced a new variable for audience, because it can differ from zeebeApi, and added examples. Maybe it's better now?

@akeller
Copy link
Member

akeller commented May 30, 2023

@berndruecker @jenshb is this still being worked?

@christinaausley
Copy link
Contributor

@berndruecker @jenshb Anything I can assist with here?

@berndruecker
Copy link
Member

I will merge it now - it makes total sense. @jenshb: Thanks for the improvement of the docs!

berndruecker
berndruecker previously approved these changes Jul 21, 2023
@christinaausley
Copy link
Contributor

@jenshb I think this change also needs to be applied to the next version of docs, then we can merge. Let me know if you need any assistance with this @jenshb!

@christinaausley
Copy link
Contributor

@jenshb @berndruecker Do we just need to add this change to versioned docs? Happy to assist with this and merge, but let me know if you were planning to make any other changes here.

@jenshb
Copy link
Contributor Author

jenshb commented Aug 4, 2023

@christinaausley I added the changes from the /version_docs directory to the /docs directory. I hope that's what you intended?

Copy link
Contributor

@christinaausley christinaausley left a comment

Choose a reason for hiding this comment

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

Thanks all! I'll enable auto-merge and we can wrap this up.

@christinaausley christinaausley enabled auto-merge (squash) August 7, 2023 12:57
@christinaausley christinaausley merged commit 169e852 into camunda:main Aug 7, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:docs Documentation improvements, including new or updated content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants