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

Expose async send #81

Closed
wants to merge 1 commit into from
Closed

Expose async send #81

wants to merge 1 commit into from

Conversation

sanders41
Copy link
Contributor

Closes #71

This is to exposes the async send to external callers. By allowing this, when async programs are running they use their own even loop rather than having 2 loops start. In my testing this solved the issue in #71.

I left the send option also so non-async programs (ie fidesctl with Click) can still call it as they have been.

@sanders41
Copy link
Contributor Author

The tests fail because the api is never installed and there is not information for how to install it in setup.py. Because of this it can't be found as a module. This is unrelated to this PR.

Copy link
Contributor

@eastandwestwind eastandwestwind left a comment

Choose a reason for hiding this comment

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

Thanks for this @sanders41 !

@TheAndrewJackson
Copy link
Contributor

@sanders41 Should this be merged still or has the PR gone stale?

@sanders41
Copy link
Contributor Author

It would be good, but we put in a work around to still use the function without this PR on the fidesops side so we got the same functionality without it.

@TheAndrewJackson
Copy link
Contributor

I see. Does it make sense to merge it in because the work is already done or would it just add dead code that won't be used?

@sanders41
Copy link
Contributor Author

What we have now is working so I think it is fine to just close this.

@sanders41 sanders41 closed this Oct 21, 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.

Handling 500 Errors
3 participants