add HOT-RELOAD feature and tests#261
add HOT-RELOAD feature and tests#261apavanello wants to merge 2 commits intoAdmiral-Piett:masterfrom
Conversation
|
@apavanello Nice, this looks awesome. Just to clarify here - all you're enabling for Hot Reload is the queues and the topics themselves, correct? The other things, Log File/Region/QueueAttributes/etc. those look omitted from |
cc2c103 to
a43d9fd
Compare
|
@apavanello This change looks good, but it looks like it needs a rebase now. I have been playing with it today and it looks like this conflicts with some of the tests. I think they're treading on each other's memory addresses. Anyway - I have a branch locally with some refactoring. I'll push it when I can if you want to see - though it will be significant, not all particularly related to your code. Thought, if you could share your k8 use case that would be really helpful - I want to make sure whatever I come up with matches your needs too. In the meantime, if you want to rebase this and get it working that works too and I'll merge it. Up to you! Let me know! |
| if !ok { | ||
| return | ||
| } | ||
| if event.Has(fsnotify.Remove) { |
There was a problem hiding this comment.
After looking at this a bit deeper, I don't think I fully understand this block. Is it because in your environment - this "remove" event is what happens on a file change (in a "remove" then "recreate" pattern)? If so, the file name isn't changing is it? Could we not just do the wait/loop to check if it's rebuilt and then proceed down with the rest of the reloading calls?
I think I'm confused about 2 things really.
- Why call
StartWatcheragain, and synchronously? - Why block the outer function with the
quitchannel? In most of these non-k8 with the remove event cases would that ever actually hang up or is that goroutine just orphaned now?
|
Hello there. But I can tell you that in the case of |
|
Ah, I can totally understand that alternate project business, story of my life. 😀 I have a sizeable PR that I'll probably want to incorporate anyway, but I'll incorporate whatever you come up with too. Alternately you can review my code when it's ready. I'm fleshing out some tests but I think it's nearly ready. I'll play with the k8 use case to make sure I'm capturing it to. Thanks for the info! |
|
@apavanello So - I am throwing this out there as a new PR. The more I got into it, the more I realized it's a major feature. but my PR has your commits in there too. I want to do a few things on this since operating in this memory space is just hard. I would say - if you want to take yours over the line, go for it. I will rebase, if not, I'll do what I can. Also, if you have the bandwidth to keep an eye on it for your own use cases I'd appreciate it. |
#260