You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by adfel70 <gi...@git.apache.org> on 2018/01/15 12:40:21 UTC

[GitHub] nifi pull request #2405: NIFI-4777 get schema by id even if not latest

GitHub user adfel70 opened a pull request:

    https://github.com/apache/nifi/pull/2405

    NIFI-4777 get schema by id even if not latest

    Thank you for submitting a contribution to Apache NiFi.
    
    In order to streamline the review of the contribution we ask you
    to ensure the following steps have been taken:
    
    ### For all changes:
    - [X ] Is there a JIRA ticket associated with this PR? Is it referenced 
         in the commit message?
    
    - [X ] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
    
    - [X ] Has your PR been rebased against the latest commit within the target branch (typically master)?
    
    - [X ] Is your initial contribution a single, squashed commit?
    
    ### For code changes:
    - [ ] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
    - [ ] Have you written or updated unit tests to verify your changes?
    - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? 
    - [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
    - [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
    - [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?
    
    ### For documentation related changes:
    - [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
    
    ### Note:
    Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/adfel70/nifi NIFI-4777

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/nifi/pull/2405.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2405
    
----
commit 87f318975c1b46302e809c467295ab17b90efda1
Author: Internet <in...@...>
Date:   2018-01-15T12:34:41Z

    get schema by id even if not latest

----


---

[GitHub] nifi issue #2405: NIFI-4777 get schema by id even if not latest

Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on the issue:

    https://github.com/apache/nifi/pull/2405
  
    Hey @adfel70 I think I was looking at this PR the wrong way. I didn't notice the addition of the POST request, which is very vital. Because of that, I thought this was making a very large number of requests, but after reviewing the confluent API docs and reviewing the code again it all makes sense. The caching in this layer is different than the caching at the higher layer but was only really necessary for the routine that was there previously, before the POST request. I think your solution handles this case much better than the previous solution. And more importantly your solution appears to be correct :) I was able to test locally and see that if i changed the schema reference to an older version of an existing schema that things work correctly with this PR and did not without the PR. So I've merged this to master. Thanks for the fix and for the patience in getting it all sorted out!


---

[GitHub] nifi issue #2405: NIFI-4777 get schema by id even if not latest

Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on the issue:

    https://github.com/apache/nifi/pull/2405
  
    @adfel70 thanks for the contribution! I'm a bit concerned about the ramifications of this change as-is, though. If I'm understanding the code change, it appears to be removing all caching from the controller service. I don't think that's what we want to do, as network problems or having the Confluent Schema Registry go down would cause the dataflow to stop. I think if we want to avoid the caching mechanism, then we would need to expose a property to allow the user to choose whether or not they want to do so - or how long they want to cache the schema.


---

[GitHub] nifi issue #2405: NIFI-4777 get schema by id even if not latest

Posted by adfel70 <gi...@git.apache.org>.
Github user adfel70 commented on the issue:

    https://github.com/apache/nifi/pull/2405
  
    Hey could anyone else help me with this?
    if it is compulsory i can return the second cache


---

[GitHub] nifi issue #2405: NIFI-4777 get schema by id even if not latest

Posted by adfel70 <gi...@git.apache.org>.
Github user adfel70 commented on the issue:

    https://github.com/apache/nifi/pull/2405
  
    @markap14,
    The schema registry still has a cache in [here](https://github.com/apache/nifi/blob/7c1ce172232d5fd8ab2a1c1649a9dcbf1a9d08d7/nifi-nar-bundles/nifi-confluent-platform-bundle/nifi-confluent-schema-registry-service/src/main/java/org/apache/nifi/confluent/schemaregistry/client/CachingSchemaRegistryClient.java#L28).
    There were previously 2 caches, that was the reason i removed the ones in the rest client.


---

[GitHub] nifi pull request #2405: NIFI-4777 get schema by id even if not latest

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/nifi/pull/2405


---