Till Westmann has posted comments on this change.

Change subject: ASTERIXDB-1356: report file not found
......................................................................


Patch Set 3:

(2 comments)

https://asterix-gerrit.ics.uci.edu/#/c/771/3/asterix-external-data/src/main/java/org/apache/asterix/external/util/FileSystemWatcher.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/util/FileSystemWatcher.java:

Line 92:         } catch (HyracksDataException e) {
> This catch is not needed.
True. I put it there to avoid yet another layering of exceptions. But I agree 
that this is not relevant for this change ...


https://asterix-gerrit.ics.uci.edu/#/c/771/3/asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/util/LocalFileSystemUtils.java:

Line 67:         });
> yeah, it depends on whether we want to consider non-feed with directories a
Clearly my intent was on fixing the issue, that I got a success message with no 
results. But I also see that this might not be right for feeds. That's why I 
just returned if we have results or not from this method and leave the decision 
whether that's a problem to the caller.
I think the questions here are 
* if it's good enough to have 2 outcomes or if we need 3 outcomes (files found, 
files not found, dir doesn't exist) and 
* how we report the outcomes if we need 3 (enum vs exception?).

Thoughts?


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/771
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia12bf84adfd8442103567df42ac22e5750e1360f
Gerrit-PatchSet: 3
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Till Westmann <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Murtadha Hubail <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to