metron-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From justinleet <...@git.apache.org>
Subject [GitHub] incubator-metron pull request #421: METRON-283 Migrate Geo Enrichment outsid...
Date Wed, 25 Jan 2017 16:52:26 GMT
Github user justinleet commented on a diff in the pull request:

    https://github.com/apache/incubator-metron/pull/421#discussion_r97824391
  
    --- Diff: metron-platform/metron-enrichment/src/main/java/org/apache/metron/enrichment/bolt/GenericEnrichmentBolt.java
---
    @@ -149,9 +154,10 @@ public JSONObject load(CacheKey key) throws Exception {
         cache = CacheBuilder.newBuilder().maximumSize(maxCacheSize)
                 .expireAfterWrite(maxTimeRetain, TimeUnit.MINUTES)
                 .build(loader);
    +    GeoLiteDatabase.INSTANCE.update((String)getConfigurations().getGlobalConfig().get(GeoLiteDatabase.GEO_HDFS_FILE));
         boolean success = adapter.initializeAdapter();
    --- End diff --
    
    You're right that if we adjust the enrichment adapter to accept configuration values,
this can be pushed to the GeoAdapter, where I agree it makes more sense.
    
    Are there concerns over changing the interface method directly, or would we prefer to
give `EnrichmentAdapter` a `initializeAdapter(Map<String, Object> config)` with default
impl that calls `initializeAdapter()` with no args?  The second option makes it a bit ugly
that `GeoAdapter` still has the original `initializeAdapter()`, but has the benefit of keeping
the interface backwards compatible to anybody with custom adapters (Is this a thing that people
have?).
    
    And you are correct that there is an issue with initializing a missing DB (Great catch!).
If we make this change, I believe the only time you'd have an issue is if you set up a geo
enrichment and didn't actually set up the DB.  Which is an entirely reasonable issue to have
an error with.
    
    @dlyle65535 Would this address your concerns about having the init in the GenericEnrichmentBolt
in a satisfactory manner?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message