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

Update Python pub_sub fastapi example #635

Merged
merged 1 commit into from
May 6, 2022
Merged

Conversation

inirudebwoy
Copy link
Contributor

@inirudebwoy inirudebwoy commented Apr 4, 2022

Signed-off-by: Michał Klich michal@klichx.dev

Description

I have updated FastAPI example attached to pub_sub for Python.
It had not worked correctly.

Issue reference

Sorry, but there is no issue.

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.

@yaron2
Copy link
Member

yaron2 commented Apr 26, 2022

@inirudebwoy apologies the review has taken so long. can you please resolve the conflict?

@inirudebwoy
Copy link
Contributor Author

@yaron2 No worries. I have resolved the conflicts. There seems to be some issue with signature.

@amulyavarote
Copy link
Contributor

@inirudebwoy Can you please take the recent changes from master and rebase this with master and push the changes? And also, can you please fix DCO as well.

@inirudebwoy inirudebwoy force-pushed the master branch 4 times, most recently from c2fa222 to 4019009 Compare April 29, 2022 19:35
@inirudebwoy
Copy link
Contributor Author

@amulyavarote Sure thing. I hope it's fine now.

@amulyavarote
Copy link
Contributor

@amulyavarote Sure thing. I hope it's fine now.

Can you please fix the build errors? The changes look good to me. Lets make sure these changes work before merging by seeing the successful validations.

@inirudebwoy inirudebwoy force-pushed the master branch 2 times, most recently from ca1e26c to 7a0a2fa Compare April 30, 2022 06:31
@yaron2 yaron2 added this to the 1.8 milestone Apr 30, 2022
@inirudebwoy
Copy link
Contributor Author

inirudebwoy commented May 1, 2022 via email

@amulyavarote
Copy link
Contributor

amulyavarote commented May 2, 2022

I really like the way you test documentation 👍my code has failed on something I have not changed. Or is it related? Best regards Michał Klich

On 1 May 2022, at 1:09 AM, Yaron Schneider @.***> wrote:  Assigned #635 to @inirudebwoy. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were assigned.

I reran the failed workflows. Let's see if it succeeds.

@yaron2
Copy link
Member

yaron2 commented May 2, 2022

@amulyavarote I think mac validations are failing regardless of this PR.

@amulyavarote
Copy link
Contributor

@amulyavarote I think mac validations are failing regardless of this PR.

@yaron2 Usually rerunning the tests fixes it. I did it again. The previous PRs build passed.

@yaron2
Copy link
Member

yaron2 commented May 3, 2022

@amulyavarote I think mac validations are failing regardless of this PR.

@yaron2 Usually rerunning the tests fixes it. I did it again. The previous PRs build passed.

I think it's consistent..

@amulyavarote
Copy link
Contributor

@amulyavarote I think mac validations are failing regardless of this PR.

@yaron2 Usually rerunning the tests fixes it. I did it again. The previous PRs build passed.

I think it's consistent..

Distributed Calculator and Bindings are failing inconsistently.

@inirudebwoy
Copy link
Contributor Author

Do you need my help in solving this? I'm not able to run Github Actions on my account unfortunately.

@yaron2
Copy link
Member

yaron2 commented May 4, 2022

@amulyavarote can we merge this PR?

amulyavarote
amulyavarote previously approved these changes May 4, 2022
@amulyavarote
Copy link
Contributor

@amulyavarote can we merge this PR?

Yes!!

@amulyavarote
Copy link
Contributor

Do you need my help in solving this? I'm not able to run Github Actions on my account unfortunately.

I pushed a change. Can you please rebase now? I will merge the PR after the builds succeed.

Signed-off-by: Michał Klich <michal@klichx.dev>
@inirudebwoy
Copy link
Contributor Author

I pushed a change. Can you please rebase now? I will merge the PR after the builds succeed.

I have rebased it.

@msfussell msfussell merged commit 57fc965 into dapr:master May 6, 2022
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.

None yet

4 participants