Skip to content

Console Print url format mismatch bug fixed#803

Closed
maniram-yadav wants to merge 15 commits intotemporalio:next-serverfrom
maniram-yadav:cli_message_fix
Closed

Console Print url format mismatch bug fixed#803
maniram-yadav wants to merge 15 commits intotemporalio:next-serverfrom
maniram-yadav:cli_message_fix

Conversation

@maniram-yadav
Copy link

@maniram-yadav maniram-yadav commented May 9, 2025

Cemporal cli prints prints the server url on console when server starts server start.
Url log formats are mismatching from each other. and missing http from other place in the same url.

Sushisource and others added 15 commits February 27, 2025 10:08
)

<!--- Note to EXTERNAL Contributors -->
<!-- Thanks for opening a PR! 
If it is a significant code change, please **make sure there is an open
issue** for this.
We work best with you when we have accepted the idea first before you
code. -->

<!--- For ALL Contributors 👇 -->

## What was changed
<!-- Describe what has changed in this PR -->
Add extended info to the DescribeWorkflow output.
## Why?
<!-- Tell your future self why have you made these changes -->
Customer request.
…-with-start` commands (temporalio#762)

added `temporal workflow start-update-with-start` and `temporal workflow
execute-update-with-start` commands

`temporal workflow start-update-with-start` usage: 
```
    temporal workflow start-update-with-start \
      --update-name YourUpdate \
      --update-input '{"update-key": "update-value"}' \
      --update-wait-for-stage accepted \
      --workflow-id YourWorkflowId \
      --type YourWorkflowType \
      --task-queue YourTaskQueue \
      --id-conflict-policy Fail \
      --input '{"wf-key": "wf-value"}'
  ```


`temporal workflow execute-update-with-start` usage:
  ```
    temporal workflow execute-update-with-start \
      --update-name YourUpdate \
      --update-input '{"update-key": "update-value"}' \
      --workflow-id YourWorkflowId \
      --type YourWorkflowType \
      --task-queue YourTaskQueue \
      --id-conflict-policy Fail \
      --input '{"wf-key": "wf-value"}'
  ```

1. Closes temporalio#664

2. How was this tested:
<!--- Please describe how you tested your changes/how we can test them -->

3. Any docs updates needed?
Yes
## What was changed
quote attribute type in error message 

## Why?
To make whitespace more obvious

---------

Co-authored-by: Rodrigo Zhou <2068124+rodrigozhou@users.noreply.github.com>
<!--- Note to EXTERNAL Contributors -->
<!-- Thanks for opening a PR! 
If it is a significant code change, please **make sure there is an open
issue** for this.
We work best with you when we have accepted the idea first before you
code. -->

<!--- For ALL Contributors 👇 -->

## What was changed
<!-- Describe what has changed in this PR -->
Check if extended info is not nil.

## Why?
<!-- Tell your future self why have you made these changes -->
temporalio#771
Code assumes that some extended info exists in proto. Which is not true
for older server versions..
## What was changed
<!-- Describe what has changed in this PR -->
Pinned v1.34.1 for modernc/sqlite

## Why?
<!-- Tell your future self why have you made these changes -->
1.34.2 has a regression, waiting on
https://gitlab.com/cznic/sqlite/-/issues/196 to be resolved.
See temporalio/temporal#7333 for more details

## Checklist
<!--- add/delete as needed --->

1. Closes temporalio#777

2. How was this tested:
<!--- Please describe how you tested your changes/how we can test them
-->

3. Any docs updates needed?
<!--- update README if applicable
      or point out where to update docs.temporal.io -->
## What was changed
<!-- Describe what has changed in this PR -->
Added a tags section to match what Documentation side has

## Why?
<!-- Tell your future self why have you made these changes -->
We previously just used keywords, but that could be seen as keyword
stuffing. The tags were updated on the docs side a few months ago,
changes were never updated on this side.

## Checklist
<!--- add/delete as needed --->

1. Closes <!-- add issue number here -->

2. How was this tested:
<!--- Please describe how you tested your changes/how we can test them
-->

3. Any docs updates needed?
<!--- update README if applicable
      or point out where to update docs.temporal.io -->

---------

Co-authored-by: Chad Retz <chad.retz@gmail.com>
## What was changed
<!-- Describe what has changed in this PR -->
Created a new Github Action that auto-generates and publishes a PR to
the [documentation repo](https://github.com/temporalio/documentation)
with the latest generated CLI docs.

Corresponding docs side PR is already merged,
temporalio/documentation#3525, and first PR of
the auto-generated docs is merged in
temporalio/documentation#3528.

Docs side YML:
https://github.com/temporalio/documentation/blob/main/.github/workflows/update-cli-docs.yml

## Why?
<!-- Tell your future self why have you made these changes -->
Makes keeping CLI docs up to date significantly easier.

## Checklist
<!--- add/delete as needed --->

1. Closes 

2. How was this tested:
<!--- Please describe how you tested your changes/how we can test them
-->
Tested in this PR, but there's a small chance I missed something in the
actual release trigger workflow. No way to fully test that until a new
release is cut after this action is on main.

3. Any docs updates needed?
<!--- update README if applicable
      or point out where to update docs.temporal.io -->
@maniram-yadav maniram-yadav requested a review from a team as a code owner May 9, 2025 14:38
@CLAassistant
Copy link

CLAassistant commented May 9, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
6 out of 7 committers have signed the CLA.

✅ ychebotarev
✅ antlai-temporal
✅ THardy98
✅ dandavison
✅ jmbarzee
✅ yuandrew
❌ maniram-yadav
You have signed the CLA already but the status is still pending? Let us recheck it.

@maniram-yadav maniram-yadav changed the base branch from main to next-server May 9, 2025 14:41
@cretz
Copy link
Contributor

cretz commented May 9, 2025

@maniram-yadav - did you mean to have this merged into next-server branch or main branch? May want to change the PR target.

Also, the server is not actually general purpose HTTP, it's gRPC. It's not actually a URL and we don't want to set "http" as the scheme. Most (all?) SDKs accept host:port, not URL. Other URLs next to it are meant to access via traditional HTTP clients hence why they are URLs.

@maniram-yadav
Copy link
Author

@cretz
I just modified it because console terminal was not having consistent url format when starting the server on local. I hope this code will not break anything even someone is connecting through SDK, does not matter in which branch it is getting merge. Just it is for the purpose of let the console terminal have unique formatting of the url.
Let me know the right branch I can create the PR for or If it is not useful we can skip it.

Also, all you people are solving very unique problem, and I am looking for some opensource contribution to you organization, let me know if I can help you or your team on backend or on frontend implementation.

@cretz
Copy link
Contributor

cretz commented May 12, 2025

You will want to target main branch. Regardless, I don't believe we consider the way the server host:port is displayed to be a bug. It is not a URL by intention and we don't want it consistent with URLs you would use with browser or HTTP.

If you're looking for general contribution opportunities, you may be able to ask in the #contributing channel of Slack if you haven't already. I do think Go samples or maybe an existing open bug may be a better place to look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants