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

Http context propagation #16

Closed
andream16 opened this issue Oct 14, 2020 · 6 comments
Closed

Http context propagation #16

andream16 opened this issue Oct 14, 2020 · 6 comments
Labels
enhancement New feature or request

Comments

@andream16
Copy link

andream16 commented Oct 14, 2020

Hey @FuzzyStatic, thanks for making this package available :).

I have one question.

Is there any reason why you chosen to not allow for context propagation when performing http requests against blizzard APIs?

This would be in certain contexts like cancellation.

I would be keen on making such changes in a PR but they will be breaking changes since the APIs will go from:

func (c *Client) getURLBody(url, namespace string) ([]byte, error) {}

To:

func (c *Client) getURLBody(ctx context.Context, url, namespace string) ([]byte, error) {}

So the release of a new version via creating a v2.0.0 would be required.

Looking forward to hearing from you :).

@FuzzyStatic
Copy link
Owner

Hey @andream16,

Thanks for reaching out. To be honest, I did not have a solid understanding of context when I first created this package and have not revisited the idea since.

I can look into creating a v2 to add this functionality

-FuzzyStatic

@FuzzyStatic FuzzyStatic added the enhancement New feature or request label Oct 14, 2020
@andream16
Copy link
Author

Thank you for getting back to me @FuzzyStatic :).

Let me know if you pick this up, otherwise feel free to tag me and I'll be happy making the changes :).

@FuzzyStatic
Copy link
Owner

@andream16 it may take me some time to pick this up, so if you want to tackle it let me know :)

@FuzzyStatic
Copy link
Owner

@andream16 try the v2 package with release version 1.1.0 for context propagation.

@andream16
Copy link
Author

Thank you @FuzzyStatic. You are a legend 💪 .

@FuzzyStatic
Copy link
Owner

Thanks! It took me a few tries to figure out how go mod does versioning and for some reason, since I redid the version 1.0.0 release a few times, go mod pulls the wrong commit for that version, so I had to make a v1.0.1.

Either way, I've tested v1 and v2 on my end and it is working (with release v1.1.0). Let me know if you have any issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants