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

Jobs API Quickstart for Python - HTTP #1160

Open
wants to merge 10 commits into
base: release-1.15
Choose a base branch
from

Conversation

rochabr
Copy link
Contributor

@rochabr rochabr commented Feb 5, 2025

Description

Creating Jobs API Quickstart for Python - HTTP

Issue reference

Please reference the issue this PR will close: #1110

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • The quickstart code compiles correctly
  • You've tested new builds of the quickstart if you changed quickstart code
  • You've updated the quickstart's README if necessary
  • If you have changed the steps for a quickstart be sure that you have updated the automated validation accordingly. All of our quickstarts have annotations that allow them to be executed automatically as code. For more information see mechanical-markdown. For user guide with examples see Examples.

rochabr and others added 7 commits January 28, 2025 18:36
@rochabr rochabr requested review from a team as code owners February 5, 2025 13:48
Copy link

@alicejgibbons alicejgibbons left a comment

Choose a reason for hiding this comment

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

A few minor things :) ty!

@@ -0,0 +1,276 @@
### Flask ###

Choose a reason for hiding this comment

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

We should always have the gitignore at the root, not nested

jobs/python/http/README.md Outdated Show resolved Hide resolved
## Install dependencies

```bash
pip install -r requirements.txt

Choose a reason for hiding this comment

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

Don't the mechanical markdown tests need this to run correctly? Can you confirm they run locally?

jobs/python/http/README.md Outdated Show resolved Hide resolved
dapr run --app-id job-service --app-port 6200 --dapr-http-port 6280 -- python app.py
```

### Schedule Jobs

Choose a reason for hiding this comment

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

Suggested change
### Schedule Jobs
### Schedule jobs

jobs/python/http/dapr.yaml Show resolved Hide resolved
jobs/python/http/dapr.yaml Outdated Show resolved Hide resolved
jobs/python/http/dapr.yaml Outdated Show resolved Hide resolved
print('Received job request...', flush=True)

try:
# Check if path starts with /job/

Choose a reason for hiding this comment

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

why do we need 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.

to ignore paths paths sent to this server. Instead of enabling a /job endpoint, Python's HTTPServer accepts requests to any endpoint and you allow or deny within the method implementation.

# Parse outer JSON data
outer_data = json.loads(raw_data)

# The payload might be double-encoded, so try parsing again if it's a string

Choose a reason for hiding this comment

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

Also why do we need this if we are the ones sending the payload as well? just don't double-encode it?

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 had issues parsing this data as-is. Had to add this to make it work. I can take some time to investigate it more.

rochabr and others added 3 commits February 6, 2025 12:01
Co-authored-by: Alice Gibbons <[email protected]>
Signed-off-by: Fernando Rocha <[email protected]>
Co-authored-by: Alice Gibbons <[email protected]>
Signed-off-by: Fernando Rocha <[email protected]>
Signed-off-by: Fernando Rocha <[email protected]>
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.

3 participants