Recursively and continously watch for file change #35
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lanwen
requested changes
Sep 6, 2017
juseppe-cli/pom.xml
Outdated
| @@ -3,7 +3,7 @@ | |||
| <parent> | |||
| <artifactId>juseppe</artifactId> | |||
| <groupId>ru.lanwen.jenkins</groupId> | |||
| <version>1.1.2-SNAPSHOT</version> | |||
| <version>1.2-SNAPSHOT</version> | |||
There was a problem hiding this comment.
will be updated automatically on release, please revert
|
|
||
| key.reset(); | ||
| key = watcher.take(); | ||
| private static final Logger LOG = LoggerFactory.getLogger(WatchFiles.class); |
There was a problem hiding this comment.
please avoid indentation changes - it is hard to review actual changes in this case...
| @Override | ||
| public void run() { | ||
| LOG.info("Start to watch for changes: {}", path); | ||
| while (true) { |
There was a problem hiding this comment.
don't do that ever - always put real condition (originally key != null)
| Path dir = keys.get(key); | ||
|
|
||
| if (dir == null) { | ||
| LOG.error(String.format("%s: WatchKey not recognized!!", getClass())); |
There was a problem hiding this comment.
why !! ? :) Key should be logged too, i think
| } | ||
|
|
||
| for (WatchEvent<?> event : key.pollEvents()) { | ||
| @SuppressWarnings("rawtypes") |
There was a problem hiding this comment.
put this on the method - its not very helpful to see it in the middle of the method
| } catch (IOException e) { | ||
| throw new RuntimeException(format("Can't read path %s", pluginsDir.toAbsolutePath()), e); | ||
| } | ||
| try (Stream<Path> paths = Files.walk(pluginsDir)) { |
|
Thanks for changes, really appreciate this functionality! But please fix some style issues |
|
Also, whould you mind to add ability to disable recursive scan (to allow for example serve different plugins from different folders in one tree with different juseppes - some can already use this scheme)19:10, 6 сентября 2017 г., Valentine Nwachukwu <[email protected]>:@valdiz777 commented on this pull request.
In juseppe-core/src/main/java/ru/lanwen/jenkins/juseppe/gen/source/PathPluginSource.java:
- return StreamSupport.stream(paths.spliterator(), false).map(path -> {
- try {
- LOG.trace("Process file {}", path);
-
- return HPI.loadHPI(path.toFile())
- .withUrl(pluginsDir.relativize(path).toString());
-
- } catch (Exception e) {
- LOG.error("Fail to get the {} info", path.toAbsolutePath(), e);
- return null;
- }
- }).filter(Objects::nonNull).collect(Collectors.toList());
- } catch (IOException e) {
- throw new RuntimeException(format("Can't read path %s", pluginsDir.toAbsolutePath()), e);
- }
+ try (Stream<Path> paths = Files.walk(pluginsDir)) {
Yes it will
—You are receiving this because you commented.Reply to this email directly, view it on GitHub, or mute the thread.
|
added 2 commits
September 6, 2017 13:02
|
Updated |
lanwen
reviewed
Sep 8, 2017
| Path dir = keys.get(key); | ||
|
|
||
| if (dir == null) { | ||
| LOG.error(String.format("%s: WatchKey: %s is not recognized!", getClass(), key.toString())); |
There was a problem hiding this comment.
one more thing - sorry for not noticed it earlier
slf4j supports placeholders with {} instead of %s - so you can omit String.format
lanwen
reviewed
Sep 8, 2017
| ) { | ||
| @Override | ||
| public String resolved() { | ||
| return String.valueOf(populated().getPort()); |
There was a problem hiding this comment.
should be getRecursiveWatch i think
lanwen
approved these changes
Sep 8, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For anyone looking for this functionality. This will enable people to be able to structure their local repositories however they wish.