-
Notifications
You must be signed in to change notification settings - Fork 56
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
Updates for temporal CLI over tctl and temporal dev-server #131
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! One minor note about the env var.
encryption/README.md
Outdated
Ensure you have an environment variable set to the Namespace your Workflows are in: | ||
|
||
|
||
export TEMPORAL_NAMESPACE=<Namespace Name> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This variable is not used in this sample, the namespace is fixed to default
(we expect to come later and make all samples run somewhere configurable)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added that in because my TEMPORAL_NAMESPACE was set to my Cloud Namespace and when I ran the starter, I got an error that the Namespace didn't exist, so I reset it to "default" and then it ran fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The starter in the encryption sample doesn't use env vars. Do you maybe mean the temporal
CLI which does? There may be other env vars the CLI uses that you would have to make sure are unset if you're concerned about your existing environment variable leftovers polluting the CLI (though I can't think of any for dev server). Also, would encourage unset TEMPORAL_NAMESPACE
instead of export TEMPORAL_NAMESPACE=<Namespace Name>
and "Namespace your Workflows are in". This sample is hardcoded to work with default
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't using the CLI to start the Workflow. I do see where it is using default, so I will have to debug what I was seeing. I will remove that statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! You'll need to merge main
. Also let me know if it doesn't let you click the merge button and I'll click it.
What was changed
I updated the README to use the
temporal
CLI instead oftctl
and updated codec_server.py to default to port 8233 for the local web UI since that is the default port fortemporal server start-dev
Why?
Checklist
Closes
How was this tested:
Tested on my local machine
Done