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

fixing issue #91 #222

Merged
merged 4 commits into from
Aug 26, 2016
Merged

fixing issue #91 #222

merged 4 commits into from
Aug 26, 2016

Conversation

jdehrlich
Copy link

Added a map in the avrorestproducer that will cache schemas and return the id if there is one for that schema rather than reregistering.

@ConfluentJenkins
Copy link
Contributor

Can one of the admins verify this patch?

@ghost
Copy link

ghost commented Jul 14, 2016

@confluentinc It looks like @jdehrlich just signed our Contributor License Agreement. 👍

Always at your service,

clabot

@criccomini
Copy link

Can someone look at this? We're running it in prod already.

@criccomini
Copy link

Ping

@criccomini
Copy link

@ewencp @gwenshap could someone please have a look? We've been running this patch in prod for over a month now.

@ewencp
Copy link
Contributor

ewencp commented Aug 26, 2016

@criccomini @jdehrlich Sorry for the slow review. LGTM. We probably should get a real cache in there rather than a collection that can continue to grow, but schemas, of all things, shouldn't really cause any problems.

Thanks for the contribution!

@ewencp ewencp merged commit e87400c into confluentinc:master Aug 26, 2016
@criccomini
Copy link

@ewencp assuming this will miss 3.0.1?

@ewencp
Copy link
Contributor

ewencp commented Aug 26, 2016

@criccomini Unfortunately yes, and I apologize, that's my bad for not getting to the review earlier.

@ewencp
Copy link
Contributor

ewencp commented Aug 26, 2016

Btw, we can cherry-pick if you want, not sure what the likelihood of a 3.0.2 is though.

@blbradley
Copy link

I made a Docker image with this patch applied. You can find it on Docker Hub as an automated build.

@blbradley
Copy link

For those that need this before the next release

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.

6 participants