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

Adds a tutorial for calling an API with a web framework #166

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

rachfop
Copy link
Contributor

@rachfop rachfop commented Nov 22, 2023

Preview

What was changed

Adds a Web API and front end for users learning Temporal

Why?

Getting lots of questions on how to run API requests in Temporal

Checklist

Code is here:
https://github.com/rachfop/weather-api

  • Python code review
  • Transfer code to temporal repo
  • Snipsync code
  1. Closes

  2. How was this tested:

  1. Any docs updates needed?

Copy link

vercel bot commented Nov 22, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
temporal-learning ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 13, 2023 5:22pm

Here's what you'll explore and accomplish:

1. **Workflow (the orchestrator):** You'll delve deep into the Workflow Class, the central orchestrator of your Temporal Application. This class meticulously outlines the sequence of operations, directing the call to the weather API.
2. **Activity (the executor):** The Activity is where the action happens. This functional unit is key to performing tasks like fetching and processing data, bridging the gap between your Workflow's instructions and the external API's responses.
Copy link
Member

Choose a reason for hiding this comment

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

orchestrator and executor don't play nicely with "durable execution".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will remove these things.

forecast_data = await response.json()
periods = forecast_data["properties"]["periods"]
return periods
else:
Copy link
Member

Choose a reason for hiding this comment

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

I would differentiate between retryable and non retryable statuses and fail activity without retries in non retryable status case.

Copy link
Member

Choose a reason for hiding this comment

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

Activities in Temporal can have multiple parameters, and all passed values are recorded in the Workflow's Event History.
This structure ensures a clear separation between the execution of tasks (Activities) and their orchestration (Workflow), a fundamental principle in Temporal's architecture.

Using `async` in Temporal's Activity code is crucial for non-blocking operations, particularly for network calls like API requests.
Copy link
Member

Choose a reason for hiding this comment

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

Should we mention that activities can be implemented in sync way as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've got a note that says:

While this tutorial uses aiohttp, you can use any HTTP client library you like; however, you should choose frameworks the supports your asynchronous or synchronous calls.

For more information, see Asynchronous vs. Synchronous Activity implementations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also have the repo in a sync for (not ready for review yet):
https://github.com/rachfop/weather-api/tree/main/sync_functions


@app.route("/weather")
async def get_weather():
client = await Client.connect("localhost:7233")
Copy link
Member

Choose a reason for hiding this comment

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

I believe a Client is expensive to create. So, it should be instantiated once per process and not on each request.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will use the Client like this:

async def get_client() -> Client:
    return await Client.connect("localhost:7233")

async def get_weather():
client = await Client.connect("localhost:7233")
weather_params = WeatherParams(office="SEW", gridX=123, gridY=61)
forecast_data = await client.execute_workflow(
Copy link
Member

Choose a reason for hiding this comment

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

What is the advantage of using Temporal for this use case? I don't see any as the get_weather creates a workflow per request, never reconnects to already running workflow on failures and could be implemented without Temporal and get exactly the same semantics.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's for the activity retries, but I agree in general this does not show much benefit to using Temporal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are a few reasons, which are explained in the tutorial. I think they're strong enough for beginners to see the value of Temporal - without them drowning in information.

The rate limit is not public information, but allows a generous amount for typical use. If the rate limit is exceeded a request will return with an error, and may be retried after the limit clears (typically within 5 seconds).

So these Activities fail quite often and you never know or how.

The National Weather Service API was chosen for this project as frictionless developer experience, in terms of getting access to an API without needing additional sign up.

Users don't need to create a new account or additonal sign ups to use this.

keywords: [python,temporal, sdk, tutorial, hello world, temporal application, workflow, activity, test temporal workflows, mock temporal activities, pytest]
last_update:
date: 2024-01-01
description: In this tutorial you will build a Temporal Application that calls a web API using the Python SDK. You'll write a Workflow, an Activity, tests, and define a Worker.
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't https://learn.temporal.io/tutorials/python/data-pipelines/ also call a web API from Python? Is it just the tests that differentiate them, and if so shouldn't we add tests to the existing one? There seems to be a lot of overlap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The point of this tutorial is to take users from their first app (calling hello world) to, what we think, is the next logical step: communicating with an API.

Weather apps are really basic implementations, and users are familiar with them.

Copy link
Member

Choose a reason for hiding this comment

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

There seems to be a lot overlap, but I guess too many tutorials doesn't hurt. It can be a bit jarring to see maybe this one properly using sessions as activity state and this one using tests and that one not.

Upon completing this tutorial, you'll Temporal application and gain comprehensive understanding of the interactions between its various components.
This experience will prove invaluable as you continue to build sophisticated, scalable web applications using Temporal.

All necessary code and resources are available in the `hello-weather-python-template` repository.
Copy link
Member

Choose a reason for hiding this comment

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

Can you link to this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have it linked in the top of this PR. Will link here:
https://github.com/rachfop/weather-api

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will be adding now.


### Update Workflow imports

First, you'll need to prepare the essential tools. Open your `workflows.py` file and add the following import statements:
Copy link
Member

Choose a reason for hiding this comment

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

Wasn't immediately obvious that I needed to have files already present. I see the prerequisites mention another article, but it's not clear I needed code from there. Can you tell the user what code they are supposed to have checked out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The prerequisites calls out building a Temporal Application, but you are right it isn't clear that this is building off the previous section.
I think I'm going to change it so that everything is in isolation and this confusion won't persist.

Comment on lines +107 to +108
Now, you define the `WeatherWorkflow` class.
This class is responsible for orchestrating the execution of the `get_weather` Activity from the `WeatherActivities` class.
Copy link
Member

Choose a reason for hiding this comment

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

In consistent with whether you put multiple sentences on one line or one line per sentence

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One sentence per line, will fix.

class WeatherWorkflow:
@workflow.run
async def run(self, weather_params: WeatherParams) -> list[dict]:
forecast_periods = await workflow.execute_activity(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
forecast_periods = await workflow.execute_activity(
forecast_periods = await workflow.execute_activity_method(

Here and elsewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.


yield

await runner.cleanup()
Copy link
Member

Choose a reason for hiding this comment

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

Should probably be in a finally block

# test_activity.py
@pytest.mark.asyncio
@pytest.mark.usefixtures("start_fake_weather_service")
@pytest.mark.parametrize(
Copy link
Member

Choose a reason for hiding this comment

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

I don't think there's really any value in parameterizing this test via a decorator, just put these objects in the actual test code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

),
],
)
async def test_get_weather(input, expected_output, weather_activities):
Copy link
Member

Choose a reason for hiding this comment

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

How does the weather_activities parameter get set here? I don't see its fixture

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Redid tests. I think that addresses this.


## Test the Activity

Testing your Temporal application is an essential step to ensure the reliability and correctness of its Workflows and Activities.
Copy link
Member

Choose a reason for hiding this comment

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

But you only test the activity not the workflow

docs/getting_started/python/hello_api_in_python/index.md Outdated Show resolved Hide resolved
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.

4 participants