)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d8bfd0a02827d7e52e11976b823e74825443ccd9","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     John Schwarz \u003cjschwarz@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-10-23 17:01:00 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add ability to run daemonized Neutron processes"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The current Daemon class allows for spawning processes that are intended"},{"line_number":10,"context_line":"to be daemonized. For example, the neutron-ns-metadata-proxy uses this"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9aa7fdbe_51b482d0","line":7,"updated":"2014-10-23 17:15:52.000000000","message":"why don\u0027t you take a step back and tell us why you need to run Neutron processes as daemons?","commit_id":"bfe2e990265f83af4c3a17d436e06c4e1be783c5"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"efdf0d220040c499d01eae19ada300e9900783e2","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     John Schwarz \u003cjschwarz@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-10-23 17:01:00 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add ability to run daemonized Neutron processes"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The current Daemon class allows for spawning processes that are intended"},{"line_number":10,"context_line":"to be daemonized. For example, the neutron-ns-metadata-proxy uses this"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9aa7fdbe_d304ed19","line":7,"in_reply_to":"9aa7fdbe_4bd5abb7","updated":"2014-10-24 06:08:25.000000000","message":"When spawning a process with the current code in neutron.agent.linux.agent, the code waits for all the input/output to finish (ie. communicate()). This practically blocks the parent process unless the direct child process finishes quickly (and wait() returns).\n\nSince we\u0027re not currently interested in the stdout of the processes (stage #1 doesn\u0027t include an all-knowing test framework which can also diagnose exceptions, nor does it include a Skynet implementation ;-)), async_process doesn\u0027t fit here IMO, so Daemon was chosen.","commit_id":"bfe2e990265f83af4c3a17d436e06c4e1be783c5"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"388efdc490079e6f0cba59ad2c641094239c77bf","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     John Schwarz \u003cjschwarz@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-10-23 17:01:00 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add ability to run daemonized Neutron processes"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The current Daemon class allows for spawning processes that are intended"},{"line_number":10,"context_line":"to be daemonized. For example, the neutron-ns-metadata-proxy uses this"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9aa7fdbe_51d562b7","line":7,"in_reply_to":"9aa7fdbe_51b482d0","updated":"2014-10-23 17:22:22.000000000","message":"Basically as part of the integration tests framework, we want to be able to spawn neutron agents/services and kill them at will, along with any number of config files we want.","commit_id":"bfe2e990265f83af4c3a17d436e06c4e1be783c5"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"ce300c9a8558e26f1df0898f1006dec136aef0dd","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     John Schwarz \u003cjschwarz@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-10-23 17:01:00 +0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add ability to run daemonized Neutron processes"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The current Daemon class allows for spawning processes that are intended"},{"line_number":10,"context_line":"to be daemonized. For example, the neutron-ns-metadata-proxy uses this"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9aa7fdbe_4bd5abb7","line":7,"in_reply_to":"9aa7fdbe_51d562b7","updated":"2014-10-23 22:21:02.000000000","message":"and why would that be impossible if they weren\u0027t daemonized?","commit_id":"bfe2e990265f83af4c3a17d436e06c4e1be783c5"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d8bfd0a02827d7e52e11976b823e74825443ccd9","unresolved":false,"context_lines":[{"line_number":17,"context_line":"to run (for example, \u0027neutron.server\u0027 and \u0027main\u0027 respectively) and runs"},{"line_number":18,"context_line":"it in daemonized mode. Some minor nits are also fixed."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"The reason why simply running \u0027/usr/bin/neutron-server ... \u0026 disown\u0027"},{"line_number":21,"context_line":"won\u0027t work is that the \u0027\u0026 disown\u0027 syntax is a specific bash syntax."},{"line_number":22,"context_line":"In short, using that will require running an interactive bash shell and"},{"line_number":23,"context_line":"executing the program in that shell, which is far from optimal - so this"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9aa7fdbe_31fd3e6f","line":20,"updated":"2014-10-23 17:15:52.000000000","message":"have you considered using something like daemonize [1] that would avoid us writing all this plumbing code?\n\n[1] http://linux.die.net/man/1/daemonize","commit_id":"bfe2e990265f83af4c3a17d436e06c4e1be783c5"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"efdf0d220040c499d01eae19ada300e9900783e2","unresolved":false,"context_lines":[{"line_number":17,"context_line":"to run (for example, \u0027neutron.server\u0027 and \u0027main\u0027 respectively) and runs"},{"line_number":18,"context_line":"it in daemonized mode. Some minor nits are also fixed."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"The reason why simply running \u0027/usr/bin/neutron-server ... \u0026 disown\u0027"},{"line_number":21,"context_line":"won\u0027t work is that the \u0027\u0026 disown\u0027 syntax is a specific bash syntax."},{"line_number":22,"context_line":"In short, using that will require running an interactive bash shell and"},{"line_number":23,"context_line":"executing the program in that shell, which is far from optimal - so this"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9aa7fdbe_0e02cc09","line":20,"in_reply_to":"9aa7fdbe_0b9d630b","updated":"2014-10-24 06:08:25.000000000","message":"Will see about switching to daemonize next week.","commit_id":"bfe2e990265f83af4c3a17d436e06c4e1be783c5"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1669eb1ae0879ca4c11f99d2f71c3e46a6c97d32","unresolved":false,"context_lines":[{"line_number":17,"context_line":"to run (for example, \u0027neutron.server\u0027 and \u0027main\u0027 respectively) and runs"},{"line_number":18,"context_line":"it in daemonized mode. Some minor nits are also fixed."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"The reason why simply running \u0027/usr/bin/neutron-server ... \u0026 disown\u0027"},{"line_number":21,"context_line":"won\u0027t work is that the \u0027\u0026 disown\u0027 syntax is a specific bash syntax."},{"line_number":22,"context_line":"In short, using that will require running an interactive bash shell and"},{"line_number":23,"context_line":"executing the program in that shell, which is far from optimal - so this"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7a8c0949_9f89b228","line":20,"in_reply_to":"9aa7fdbe_0e02cc09","updated":"2014-11-07 08:18:23.000000000","message":"@Armando, it would seem that \u0027daemonize\u0027 isn\u0027t in the ubuntu repositories. I didn\u0027t look thoroughly but will do next week. If it really isn\u0027t there, it\u0027s probably best not to use it (and use the approach in this patchset instead).","commit_id":"bfe2e990265f83af4c3a17d436e06c4e1be783c5"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"388efdc490079e6f0cba59ad2c641094239c77bf","unresolved":false,"context_lines":[{"line_number":17,"context_line":"to run (for example, \u0027neutron.server\u0027 and \u0027main\u0027 respectively) and runs"},{"line_number":18,"context_line":"it in daemonized mode. Some minor nits are also fixed."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"The reason why simply running \u0027/usr/bin/neutron-server ... \u0026 disown\u0027"},{"line_number":21,"context_line":"won\u0027t work is that the \u0027\u0026 disown\u0027 syntax is a specific bash syntax."},{"line_number":22,"context_line":"In short, using that will require running an interactive bash shell and"},{"line_number":23,"context_line":"executing the program in that shell, which is far from optimal - so this"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9aa7fdbe_d1f79257","line":20,"in_reply_to":"9aa7fdbe_31fd3e6f","updated":"2014-10-23 17:22:22.000000000","message":"Yes, but I reckoned it might be easier to write this patch instead of getting a new package to the requirements of future installations. If you think using daemonize instead is a good idea, you\u0027re welcome to say and I\u0027ll reconsider this approach :)","commit_id":"bfe2e990265f83af4c3a17d436e06c4e1be783c5"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"ce300c9a8558e26f1df0898f1006dec136aef0dd","unresolved":false,"context_lines":[{"line_number":17,"context_line":"to run (for example, \u0027neutron.server\u0027 and \u0027main\u0027 respectively) and runs"},{"line_number":18,"context_line":"it in daemonized mode. Some minor nits are also fixed."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"The reason why simply running \u0027/usr/bin/neutron-server ... \u0026 disown\u0027"},{"line_number":21,"context_line":"won\u0027t work is that the \u0027\u0026 disown\u0027 syntax is a specific bash syntax."},{"line_number":22,"context_line":"In short, using that will require running an interactive bash shell and"},{"line_number":23,"context_line":"executing the program in that shell, which is far from optimal - so this"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9aa7fdbe_0b9d630b","line":20,"in_reply_to":"9aa7fdbe_d1f79257","updated":"2014-10-23 22:21:02.000000000","message":"I reckon that less code we gotta maintain the better for us. We don\u0027t need to reinvent the wheel for everything. If there\u0027s another fundamental reason why we cannot use such a utility (ie. not easily available on distros), then I don\u0027t see why we should not pull another dependency for tests.","commit_id":"bfe2e990265f83af4c3a17d436e06c4e1be783c5"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f9fa1121f28d4f42d39c1f4df66f74257e9a5877","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The current Daemon class allows for spawning processes that are intended"},{"line_number":10,"context_line":"to be daemonized. For example, the neutron-ns-metadata-proxy uses this"},{"line_number":11,"context_line":"class to be able to easily daemonized. The current implementation"},{"line_number":12,"context_line":"requires that each such process inherit from the Daemon class (like the"},{"line_number":13,"context_line":"metadata proxy does), but when trying to run valid Neutron processes"},{"line_number":14,"context_line":"(like neutron-server or the l3 agent) in a daemonized mode, a new class"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"3a961159_d1d857f1","line":11,"updated":"2014-12-21 04:34:48.000000000","message":"to easily -\u003e to be easily","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4f29c1055b06ef38553558a1eee0b3c3e289dd91","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The current Daemon class allows for spawning processes that are intended"},{"line_number":10,"context_line":"to be daemonized. For example, the neutron-ns-metadata-proxy uses this"},{"line_number":11,"context_line":"class to be able to easily daemonized. The current implementation"},{"line_number":12,"context_line":"requires that each such process inherit from the Daemon class (like the"},{"line_number":13,"context_line":"metadata proxy does), but when trying to run valid Neutron processes"},{"line_number":14,"context_line":"(like neutron-server or the l3 agent) in a daemonized mode, a new class"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"3a961159_a6c17a83","line":11,"in_reply_to":"3a961159_d1d857f1","updated":"2014-12-25 09:13:11.000000000","message":"Done","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"290064e6e14c7a926484fdc4d2eb478eaa321332","unresolved":false,"context_lines":[{"line_number":27,"context_line":" * [1/3] - Add ability to run daemonized Neutron processes"},{"line_number":28,"context_line":" - [2/3] - Add full-stack tests framework"},{"line_number":29,"context_line":" - [3/3] - Test that creating HA routers work"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"Implements: blueprint integration-tests"},{"line_number":32,"context_line":"Change-Id: If75558418bc6ea608ec88f3dd5ffdfdb22aff4a6"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"3a961159_371f445d","line":30,"updated":"2014-12-30 07:43:45.000000000","message":"This is missing a DocImpact flag\n\n\nAll new config vars should be listed and explained here","commit_id":"2f7a0c2c13b6515e5927eebf9e1493a4ba25f1a6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"7b04bd9090163a6b62fc6ac45077958c0ff9cc48","unresolved":false,"context_lines":[{"line_number":27,"context_line":" * [1/3] - Add ability to run daemonized Neutron processes"},{"line_number":28,"context_line":" - [2/3] - Add full-stack tests framework"},{"line_number":29,"context_line":" - [3/3] - Test that creating HA routers work"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"Implements: blueprint integration-tests"},{"line_number":32,"context_line":"Change-Id: If75558418bc6ea608ec88f3dd5ffdfdb22aff4a6"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"3a961159_693814d5","line":30,"in_reply_to":"3a961159_371f445d","updated":"2014-12-30 09:09:55.000000000","message":"Done","commit_id":"2f7a0c2c13b6515e5927eebf9e1493a4ba25f1a6"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"97be7bd55e83f9b85361cd0412c90e16d1e3b967","unresolved":false,"context_lines":[{"line_number":30,"context_line":" - name - an (hopefully) unique string for easier distinguishing between"},{"line_number":31,"context_line":"          other daemons."},{"line_number":32,"context_line":" - module - python path of the module to import after daemonization."},{"line_number":33,"context_line":" - main_func - the function (located in the module) to run."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"This is part of the integration tests effort:"},{"line_number":36,"context_line":" * [1/3] - Add ability to run daemonized Neutron processes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"3a961159_fc88c0c7","line":33,"updated":"2015-01-07 10:38:47.000000000","message":"and user/group?","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"3ea394f419c0623695e24841c7a9c0d40fc73248","unresolved":false,"context_lines":[{"line_number":30,"context_line":" - name - an (hopefully) unique string for easier distinguishing between"},{"line_number":31,"context_line":"          other daemons."},{"line_number":32,"context_line":" - module - python path of the module to import after daemonization."},{"line_number":33,"context_line":" - main_func - the function (located in the module) to run."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"This is part of the integration tests effort:"},{"line_number":36,"context_line":" * [1/3] - Add ability to run daemonized Neutron processes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"3a961159_8331fbee","line":33,"in_reply_to":"3a961159_fc88c0c7","updated":"2015-01-08 07:43:00.000000000","message":"yep :)","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d0ea113f32e84b273b18dccd5d2c7f247bdf35f0","unresolved":false,"context_lines":[{"line_number":15,"context_line":"must be written for each such process. This patch creates a new"},{"line_number":16,"context_line":"ProcessDaemon which allows for specifying an import patch and a function"},{"line_number":17,"context_line":"to run (for example, \u0027neutron.server\u0027 and \u0027main\u0027 respectively) and runs"},{"line_number":18,"context_line":"it in daemonized mode."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"The reason why simply running \u0027/usr/bin/neutron-server ... \u0026 disown\u0027"},{"line_number":21,"context_line":"won\u0027t work is that the \u0027\u0026 disown\u0027 syntax is a specific bash syntax."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"fa81d914_672748b7","line":18,"updated":"2015-01-29 00:23:13.000000000","message":"To be perfectly honest I don\u0027t think that the added complexity introduced here is justified by the fact that it is required to run the various Neutron components as daemons. For testing purposes, I think it\u0027s a lot cleaner if we followed the existing model and we required each of these modules to inherit the Daemon class and be used directly in the tests.\n\nSo I am advocating for a complete change of direction in this patch, where we don\u0027t really need to tweak the existing code to allow for certain functions to be daemonized as you did here. Unless you can manage to convince me otherwise.\n\nHaving said that, if all you need is to run the components like the server, or the agents at will for testing purposes, can you do that by using subprocess popen directly? What led to down this path on using the Daemon class? We should be able to spawn and kill neutron\u0027s components at will using popen","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"8a3f2eabd0bd6e54a688920a512298bb74199d2e","unresolved":false,"context_lines":[{"line_number":15,"context_line":"must be written for each such process. This patch creates a new"},{"line_number":16,"context_line":"ProcessDaemon which allows for specifying an import patch and a function"},{"line_number":17,"context_line":"to run (for example, \u0027neutron.server\u0027 and \u0027main\u0027 respectively) and runs"},{"line_number":18,"context_line":"it in daemonized mode."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"The reason why simply running \u0027/usr/bin/neutron-server ... \u0026 disown\u0027"},{"line_number":21,"context_line":"won\u0027t work is that the \u0027\u0026 disown\u0027 syntax is a specific bash syntax."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"fa81d914_8f8c5237","line":18,"in_reply_to":"fa81d914_672748b7","updated":"2015-01-29 01:28:15.000000000","message":"I was able to put this snippet together:\n\n  #!/usr/bin/env python\n  import os\n  import signal\n  import subprocess\n  import time\n\n  neutron_server  \u003d subprocess.Popen([\u0027neutron-server\u0027])  # pass your own config here\n  print \u0027Waiting before killing the server...\u0027\n  time.sleep(1)\n  print \u0027Killing....\u0027\n  time.sleep(10)\n  os.kill(neutron_server.pid, signal.SIGTERM)\n  print \u0027Killed!\u0027\n  time.sleep(10)\n\nAnd I was able to run neutron-server whilst having control in the parent process. Can you elaborate why this is not a viable alternative?","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"fbaff8c8779991dd312190309704410ebcd25ee2","unresolved":false,"context_lines":[{"line_number":15,"context_line":"must be written for each such process. This patch creates a new"},{"line_number":16,"context_line":"ProcessDaemon which allows for specifying an import patch and a function"},{"line_number":17,"context_line":"to run (for example, \u0027neutron.server\u0027 and \u0027main\u0027 respectively) and runs"},{"line_number":18,"context_line":"it in daemonized mode."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"The reason why simply running \u0027/usr/bin/neutron-server ... \u0026 disown\u0027"},{"line_number":21,"context_line":"won\u0027t work is that the \u0027\u0026 disown\u0027 syntax is a specific bash syntax."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"fa81d914_bfd764ec","line":18,"in_reply_to":"fa81d914_8f8c5237","updated":"2015-01-29 16:22:51.000000000","message":"The reason I went down this road is twofold: first, I didn\u0027t want to invoke Popen directly as the code already contains numerous wrappers around it, and adding another specialized function to use it specifically as you suggested seemed wrong, and second, since the code base already contains the Daemon class it seemed like the right choice to make using it, so I preferred to use the Daemon class. However, I wanted to avoid the obvious code duplication so decided to extend the class instead.\n\nOverall though you\u0027re right - this patch is raising a lot of issues that are too hard to handle (specifically the chdir issue) - it looks more and more like the right solution is simplifying the solution and implementing it similarly to what you wrote in your second comment, though I do wish to see if I can still use the Daemon class as a basis (since I really don\u0027t want to add yet another Popen wrapper that has to be maintained and daemonizing seems like the better choice).\n\nI\u0027ll have a think about it and hopefully I\u0027ll be able to come up with a good design sometime next week (after my Monday\u0027s final).","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"f6985cb707cabf3c9e5f101bef8888d5ee792aa7","unresolved":false,"context_lines":[{"line_number":23,"context_line":"executing the program in that shell, which is far from optimal - so this"},{"line_number":24,"context_line":"solution was chosen instead."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"A new executable, neutron-daemon, is added. It accepts the following"},{"line_number":27,"context_line":"options:"},{"line_number":28,"context_line":" - pid_file - path to save the PID of the daemonized process."},{"line_number":29,"context_line":" - daemonize - True/False, indicating whether or not to daemonize."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"fa81d914_a1a6900e","line":26,"updated":"2015-01-28 22:33:30.000000000","message":"this looks like it\u0027s stale from a previous patchset.","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"fbaff8c8779991dd312190309704410ebcd25ee2","unresolved":false,"context_lines":[{"line_number":23,"context_line":"executing the program in that shell, which is far from optimal - so this"},{"line_number":24,"context_line":"solution was chosen instead."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"A new executable, neutron-daemon, is added. It accepts the following"},{"line_number":27,"context_line":"options:"},{"line_number":28,"context_line":" - pid_file - path to save the PID of the daemonized process."},{"line_number":29,"context_line":" - daemonize - True/False, indicating whether or not to daemonize."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"fa81d914_df05f87a","line":26,"in_reply_to":"fa81d914_a1a6900e","updated":"2015-01-29 16:22:51.000000000","message":"It is.","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"}],"neutron/agent/linux/daemon.py":[{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"bd96497702dda1e85cf69b94c0bbf9b2f51931ae","unresolved":false,"context_lines":[{"line_number":201,"context_line":"            daemon.run()"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"class ProcessDaemon(Daemon):"},{"line_number":205,"context_line":"    \"\"\"A generic Daemon class used to execute specific Neutron processes."},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    For example, we can run the l3 agent using:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_3b95a2e4","line":204,"updated":"2014-10-19 11:19:19.000000000","message":"Write unit tests for this class.","commit_id":"8287d2078a30f0da326456ccb8b9c05f683cb9cf"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f9fa1121f28d4f42d39c1f4df66f74257e9a5877","unresolved":false,"context_lines":[{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    EXTRA_OPTS \u003d []"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def __init__(self, pid_file, stdin\u003d\u0027/dev/null\u0027, stdout\u003d\u0027/dev/null\u0027,"},{"line_number":99,"context_line":"                 stderr\u003d\u0027/dev/null\u0027, procname\u003d\u0027python\u0027, uuid\u003dNone):"},{"line_number":100,"context_line":"        self.stdin \u003d stdin"},{"line_number":101,"context_line":"        self.stdout \u003d stdout"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_91ea4f72","line":98,"updated":"2014-12-21 04:34:48.000000000","message":"Please try to avoid these kinds of unrelated non-functional changes.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4f29c1055b06ef38553558a1eee0b3c3e289dd91","unresolved":false,"context_lines":[{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    EXTRA_OPTS \u003d []"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def __init__(self, pid_file, stdin\u003d\u0027/dev/null\u0027, stdout\u003d\u0027/dev/null\u0027,"},{"line_number":99,"context_line":"                 stderr\u003d\u0027/dev/null\u0027, procname\u003d\u0027python\u0027, uuid\u003dNone):"},{"line_number":100,"context_line":"        self.stdin \u003d stdin"},{"line_number":101,"context_line":"        self.stdout \u003d stdout"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_1b0387bf","line":98,"in_reply_to":"3a961159_91ea4f72","updated":"2014-12-25 09:13:11.000000000","message":"It bothered me back when I wrote this code, but not enough to change it throughout... Guess I\u0027m reverting this either way :)","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f9fa1121f28d4f42d39c1f4df66f74257e9a5877","unresolved":false,"context_lines":[{"line_number":101,"context_line":"        self.stdout \u003d stdout"},{"line_number":102,"context_line":"        self.stderr \u003d stderr"},{"line_number":103,"context_line":"        self.procname \u003d procname"},{"line_number":104,"context_line":"        self.pidfile \u003d Pidfile(pid_file, procname, uuid)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    def _fork(self):"},{"line_number":107,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_b102eb3b","line":104,"updated":"2014-12-21 04:34:48.000000000","message":"Why is the parameter changed to pid_file but the instance variable remains pidfile?","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f9fa1121f28d4f42d39c1f4df66f74257e9a5877","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        cfg.CONF.register_cli_opts(cls.EXTRA_OPTS)"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        cfg.CONF(project\u003d\u0027neutron\u0027, default_config_files\u003d[])"},{"line_number":179,"context_line":"        cfg.CONF.set_override(\u0027verbose\u0027, True)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"        if cfg.CONF.log_dir:"},{"line_number":182,"context_line":"            fmt \u003d cfg.CONF.name + \"-%Y-%m-%d--%H%M%S.log\""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_8cf5d8ee","line":179,"updated":"2014-12-21 04:34:48.000000000","message":"I don\u0027t think it\u0027s desirable to force verbose logging. If required, please limit the scope to individual tests.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4f29c1055b06ef38553558a1eee0b3c3e289dd91","unresolved":false,"context_lines":[{"line_number":176,"context_line":"        cfg.CONF.register_cli_opts(cls.EXTRA_OPTS)"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        cfg.CONF(project\u003d\u0027neutron\u0027, default_config_files\u003d[])"},{"line_number":179,"context_line":"        cfg.CONF.set_override(\u0027verbose\u0027, True)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"        if cfg.CONF.log_dir:"},{"line_number":182,"context_line":"            fmt \u003d cfg.CONF.name + \"-%Y-%m-%d--%H%M%S.log\""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_bbf353e6","line":179,"in_reply_to":"3a961159_8cf5d8ee","updated":"2014-12-25 09:13:11.000000000","message":"Done","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f9fa1121f28d4f42d39c1f4df66f74257e9a5877","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        cfg.CONF(project\u003d\u0027neutron\u0027, default_config_files\u003d[])"},{"line_number":179,"context_line":"        cfg.CONF.set_override(\u0027verbose\u0027, True)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"        if cfg.CONF.log_dir:"},{"line_number":182,"context_line":"            fmt \u003d cfg.CONF.name + \"-%Y-%m-%d--%H%M%S.log\""},{"line_number":183,"context_line":"            log_file \u003d timeutils.strtime(fmt\u003dfmt)"},{"line_number":184,"context_line":"            cfg.CONF.set_override(\u0027log_file\u0027, log_file)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_ece3bc31","line":181,"updated":"2014-12-21 04:34:48.000000000","message":"Why is this configuration necessary?  I\u0027m not sure of it\u0027s value outside of a test environment.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4f29c1055b06ef38553558a1eee0b3c3e289dd91","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        cfg.CONF(project\u003d\u0027neutron\u0027, default_config_files\u003d[])"},{"line_number":179,"context_line":"        cfg.CONF.set_override(\u0027verbose\u0027, True)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"        if cfg.CONF.log_dir:"},{"line_number":182,"context_line":"            fmt \u003d cfg.CONF.name + \"-%Y-%m-%d--%H%M%S.log\""},{"line_number":183,"context_line":"            log_file \u003d timeutils.strtime(fmt\u003dfmt)"},{"line_number":184,"context_line":"            cfg.CONF.set_override(\u0027log_file\u0027, log_file)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_5bb26f07","line":181,"in_reply_to":"3a961159_ece3bc31","updated":"2014-12-25 09:13:11.000000000","message":"You\u0027re right. I\u0027ll see about moving this to the testing framework so it will be able to configure log_file manually.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f9fa1121f28d4f42d39c1f4df66f74257e9a5877","unresolved":false,"context_lines":[{"line_number":202,"context_line":"            daemon.run()"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"class ProcessDaemon(Daemon):"},{"line_number":206,"context_line":"    \"\"\"A generic Daemon class used to execute specific Neutron processes."},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    For example, we can run the l3 agent using:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_0c8ac863","line":205,"updated":"2014-12-21 04:34:48.000000000","message":"Does this class belong outside of the test tree?","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4f29c1055b06ef38553558a1eee0b3c3e289dd91","unresolved":false,"context_lines":[{"line_number":202,"context_line":"            daemon.run()"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"class ProcessDaemon(Daemon):"},{"line_number":206,"context_line":"    \"\"\"A generic Daemon class used to execute specific Neutron processes."},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    For example, we can run the l3 agent using:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_bbe193fe","line":205,"in_reply_to":"3a961159_0c8ac863","updated":"2014-12-25 09:13:11.000000000","message":"Yes. Changing this code this way also saves quite a bit of code on the metadata_proxy\u0027s initialisation code. Moving this to the testing library will create code duplication (and we all know how much we love to avoid it), so this should be shared.\n\nBesides, somewhere down the line someone might like this and want to use it, and I want to save them moving this outside the testing modules if they do :)","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"290064e6e14c7a926484fdc4d2eb478eaa321332","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"import eventlet"},{"line_number":22,"context_line":"eventlet.monkey_patch()"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from oslo.config import cfg"},{"line_number":25,"context_line":"from oslo.utils import importutils"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_7c529369","line":23,"updated":"2014-12-30 07:43:45.000000000","message":"why the new line?","commit_id":"2f7a0c2c13b6515e5927eebf9e1493a4ba25f1a6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"7b04bd9090163a6b62fc6ac45077958c0ff9cc48","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"import eventlet"},{"line_number":22,"context_line":"eventlet.monkey_patch()"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from oslo.config import cfg"},{"line_number":25,"context_line":"from oslo.utils import importutils"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_7752ac69","line":23,"in_reply_to":"3a961159_7c529369","updated":"2014-12-30 09:09:55.000000000","message":"Done","commit_id":"2f7a0c2c13b6515e5927eebf9e1493a4ba25f1a6"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"290064e6e14c7a926484fdc4d2eb478eaa321332","unresolved":false,"context_lines":[{"line_number":158,"context_line":"            sys.exit(1)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        # Start the daemon"},{"line_number":161,"context_line":"        LOG.info(_LI(\"Daemonizing...\"))"},{"line_number":162,"context_line":"        self.daemonize()"},{"line_number":163,"context_line":"        self.run()"},{"line_number":164,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_9c4f3fbe","line":161,"updated":"2014-12-30 07:43:45.000000000","message":"what if the flag is False?","commit_id":"2f7a0c2c13b6515e5927eebf9e1493a4ba25f1a6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"7b04bd9090163a6b62fc6ac45077958c0ff9cc48","unresolved":false,"context_lines":[{"line_number":158,"context_line":"            sys.exit(1)"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        # Start the daemon"},{"line_number":161,"context_line":"        LOG.info(_LI(\"Daemonizing...\"))"},{"line_number":162,"context_line":"        self.daemonize()"},{"line_number":163,"context_line":"        self.run()"},{"line_number":164,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_77d9ccf3","line":161,"in_reply_to":"3a961159_9c4f3fbe","updated":"2014-12-30 09:09:55.000000000","message":"If the flag is False, run() will be ran instead and this function will never run. see L191.","commit_id":"2f7a0c2c13b6515e5927eebf9e1493a4ba25f1a6"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"290064e6e14c7a926484fdc4d2eb478eaa321332","unresolved":false,"context_lines":[{"line_number":204,"context_line":"    running the same module more than once (when using the \u0027ps\u0027 command,"},{"line_number":205,"context_line":"    for example)."},{"line_number":206,"context_line":"    \"\"\""},{"line_number":207,"context_line":"    EXTRA_OPTS \u003d ["},{"line_number":208,"context_line":"        cfg.StrOpt(\u0027name\u0027,"},{"line_number":209,"context_line":"                   help\u003d_(\u0027A name used to describe this process.\u0027)),"},{"line_number":210,"context_line":"        cfg.StrOpt(\u0027module\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_973558dd","line":207,"updated":"2014-12-30 07:43:45.000000000","message":"i do not think that these should be in the global section of the config file","commit_id":"2f7a0c2c13b6515e5927eebf9e1493a4ba25f1a6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"7b04bd9090163a6b62fc6ac45077958c0ff9cc48","unresolved":false,"context_lines":[{"line_number":204,"context_line":"    running the same module more than once (when using the \u0027ps\u0027 command,"},{"line_number":205,"context_line":"    for example)."},{"line_number":206,"context_line":"    \"\"\""},{"line_number":207,"context_line":"    EXTRA_OPTS \u003d ["},{"line_number":208,"context_line":"        cfg.StrOpt(\u0027name\u0027,"},{"line_number":209,"context_line":"                   help\u003d_(\u0027A name used to describe this process.\u0027)),"},{"line_number":210,"context_line":"        cfg.StrOpt(\u0027module\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_37e744ae","line":207,"in_reply_to":"3a961159_973558dd","updated":"2014-12-30 09:09:55.000000000","message":"These are CLI opts (L174), so they shouldn\u0027t appear in the global section of the config file.","commit_id":"2f7a0c2c13b6515e5927eebf9e1493a4ba25f1a6"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"290064e6e14c7a926484fdc4d2eb478eaa321332","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                   help\u003d_(\u0027A name used to describe this process.\u0027)),"},{"line_number":210,"context_line":"        cfg.StrOpt(\u0027module\u0027,"},{"line_number":211,"context_line":"                   help\u003d_(\u0027Where to look for a main function after \u0027"},{"line_number":212,"context_line":"                       \u0027daemonization. Example: neutron.agent.linux.daemon\u0027)),"},{"line_number":213,"context_line":"        cfg.StrOpt(\u0027main_func\u0027,"},{"line_number":214,"context_line":"                   default\u003d\u0027main\u0027,"},{"line_number":215,"context_line":"                   help\u003d_(\u0027Name of the main function within the module.\u0027)),"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_dc49c7b8","line":212,"updated":"2014-12-30 07:43:45.000000000","message":"indentation is dodgy","commit_id":"2f7a0c2c13b6515e5927eebf9e1493a4ba25f1a6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"7b04bd9090163a6b62fc6ac45077958c0ff9cc48","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                   help\u003d_(\u0027A name used to describe this process.\u0027)),"},{"line_number":210,"context_line":"        cfg.StrOpt(\u0027module\u0027,"},{"line_number":211,"context_line":"                   help\u003d_(\u0027Where to look for a main function after \u0027"},{"line_number":212,"context_line":"                       \u0027daemonization. Example: neutron.agent.linux.daemon\u0027)),"},{"line_number":213,"context_line":"        cfg.StrOpt(\u0027main_func\u0027,"},{"line_number":214,"context_line":"                   default\u003d\u0027main\u0027,"},{"line_number":215,"context_line":"                   help\u003d_(\u0027Name of the main function within the module.\u0027)),"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_d7f7e057","line":212,"in_reply_to":"3a961159_dc49c7b8","updated":"2014-12-30 09:09:55.000000000","message":"Is this better?","commit_id":"2f7a0c2c13b6515e5927eebf9e1493a4ba25f1a6"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"fa5a4a8258bfc47977a366354f11719020c9b62b","unresolved":false,"context_lines":[{"line_number":207,"context_line":"    \"\"\""},{"line_number":208,"context_line":"    EXTRA_OPTS \u003d ["},{"line_number":209,"context_line":"        cfg.StrOpt("},{"line_number":210,"context_line":"            \u0027name\u0027,"},{"line_number":211,"context_line":"            help\u003d_(\u0027A name used to describe this process.\u0027)),"},{"line_number":212,"context_line":"        cfg.StrOpt("},{"line_number":213,"context_line":"            \u0027module\u0027,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_356a641a","line":210,"updated":"2015-01-07 02:27:00.000000000","message":"(nit) Yuck.  I think garyk\u0027s comment was to line up the content to the opening parenthesis rather than this, please fix.\n\n(not-a-nit) Please set required\u003dTrue for each opt instead of validating arguments manually.  This means that check_arguments can probably go away.","commit_id":"7509e1f4f64e05d453b2d577e728ec2cb46fe9d6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"65bc3f2a02936a680a29321d4caad915448091c7","unresolved":false,"context_lines":[{"line_number":207,"context_line":"    \"\"\""},{"line_number":208,"context_line":"    EXTRA_OPTS \u003d ["},{"line_number":209,"context_line":"        cfg.StrOpt("},{"line_number":210,"context_line":"            \u0027name\u0027,"},{"line_number":211,"context_line":"            help\u003d_(\u0027A name used to describe this process.\u0027)),"},{"line_number":212,"context_line":"        cfg.StrOpt("},{"line_number":213,"context_line":"            \u0027module\u0027,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_59eba6a3","line":210,"in_reply_to":"3a961159_356a641a","updated":"2015-01-07 09:40:01.000000000","message":"Done and Done. Didn\u0027t know required\u003dTrue does this for me :)","commit_id":"7509e1f4f64e05d453b2d577e728ec2cb46fe9d6"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"fa5a4a8258bfc47977a366354f11719020c9b62b","unresolved":false,"context_lines":[{"line_number":239,"context_line":""},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"def main():"},{"line_number":242,"context_line":"    ProcessDaemon.main()"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_985eefe7","line":242,"updated":"2015-01-07 02:27:00.000000000","message":"Consider adding:\n\nif __name__ \u003d\u003d \u0027__main__\u0027:\n    main()\n\nThis would allow invocation with -m instead of -c:\n\npython -m neutron.agent.linux.daemon [args]\n\nAs per my comment on setup.cfg, more work is required to make this approach viable via entry points from a security perspective, so I don\u0027t think it\u0027s a problem to deviate from how the other commands are defined.","commit_id":"7509e1f4f64e05d453b2d577e728ec2cb46fe9d6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"65bc3f2a02936a680a29321d4caad915448091c7","unresolved":false,"context_lines":[{"line_number":239,"context_line":""},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"def main():"},{"line_number":242,"context_line":"    ProcessDaemon.main()"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_d95b1684","line":242,"in_reply_to":"3a961159_985eefe7","updated":"2015-01-07 09:40:01.000000000","message":"Done. Using this method won\u0027t be visible in this patchset, but later on full-stack will invoke this code with this method.","commit_id":"7509e1f4f64e05d453b2d577e728ec2cb46fe9d6"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"97be7bd55e83f9b85361cd0412c90e16d1e3b967","unresolved":false,"context_lines":[{"line_number":21,"context_line":"import sys"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"import eventlet"},{"line_number":24,"context_line":"eventlet.monkey_patch()"},{"line_number":25,"context_line":"from oslo.config import cfg"},{"line_number":26,"context_line":"from oslo.utils import importutils"},{"line_number":27,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_7cbcd06c","line":24,"updated":"2015-01-07 10:38:47.000000000","message":"monkey_patch should be done as early as possible in module imports","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"3ea394f419c0623695e24841c7a9c0d40fc73248","unresolved":false,"context_lines":[{"line_number":21,"context_line":"import sys"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"import eventlet"},{"line_number":24,"context_line":"eventlet.monkey_patch()"},{"line_number":25,"context_line":"from oslo.config import cfg"},{"line_number":26,"context_line":"from oslo.utils import importutils"},{"line_number":27,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_5040b7a4","line":24,"in_reply_to":"3a961159_7cbcd06c","updated":"2015-01-08 07:43:00.000000000","message":"Following convention set by other modules running monkey_patch(), it\u0027s done after all the std imports.","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"97be7bd55e83f9b85361cd0412c90e16d1e3b967","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    def delete_pid(self):"},{"line_number":202,"context_line":"        pidfile \u003d str(self.pidfile)"},{"line_number":203,"context_line":"        if os.path.exists(pidfile):"},{"line_number":204,"context_line":"            os.remove(pidfile)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    def handle_sigterm(self, signum, frame):"},{"line_number":207,"context_line":"        LOG.info(_LI(\"Received SIGTERM! Exiting!\"))"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_0f48f02b","line":204,"updated":"2015-01-07 10:38:47.000000000","message":"the following is thread-safe:\n\n  try:\n    os.remove(...)\n  except OSError:\n    pass","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"3ea394f419c0623695e24841c7a9c0d40fc73248","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    def delete_pid(self):"},{"line_number":202,"context_line":"        pidfile \u003d str(self.pidfile)"},{"line_number":203,"context_line":"        if os.path.exists(pidfile):"},{"line_number":204,"context_line":"            os.remove(pidfile)"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    def handle_sigterm(self, signum, frame):"},{"line_number":207,"context_line":"        LOG.info(_LI(\"Received SIGTERM! Exiting!\"))"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_100aaf78","line":204,"in_reply_to":"3a961159_0f48f02b","updated":"2015-01-08 07:43:00.000000000","message":"Done","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"97be7bd55e83f9b85361cd0412c90e16d1e3b967","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        start() will call this method after the process has daemonized."},{"line_number":230,"context_line":"        \"\"\""},{"line_number":231,"context_line":"        drop_privileges(self.user, self.group)"},{"line_number":232,"context_line":"        pass"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    @classmethod"},{"line_number":235,"context_line":"    def configure(cls):"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_af543cc6","line":232,"updated":"2015-01-07 10:38:47.000000000","message":"pass is useless","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"3ea394f419c0623695e24841c7a9c0d40fc73248","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        start() will call this method after the process has daemonized."},{"line_number":230,"context_line":"        \"\"\""},{"line_number":231,"context_line":"        drop_privileges(self.user, self.group)"},{"line_number":232,"context_line":"        pass"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    @classmethod"},{"line_number":235,"context_line":"    def configure(cls):"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_d0b747d6","line":232,"in_reply_to":"3a961159_af543cc6","updated":"2015-01-08 07:43:00.000000000","message":"Done","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"97be7bd55e83f9b85361cd0412c90e16d1e3b967","unresolved":false,"context_lines":[{"line_number":267,"context_line":"                   help\u003d_(\u0027Where to look for a main function after \u0027"},{"line_number":268,"context_line":"                          \u0027daemonization. Example: \u0027"},{"line_number":269,"context_line":"                          \u0027neutron.agent.linux.daemon\u0027)),"},{"line_number":270,"context_line":"        cfg.StrOpt(\u0027main_func\u0027,"},{"line_number":271,"context_line":"                   default\u003d\u0027main\u0027,"},{"line_number":272,"context_line":"                   help\u003d_(\u0027Name of the main function within the module.\u0027)),"},{"line_number":273,"context_line":"    ]"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_0f3e30b8","line":270,"updated":"2015-01-07 10:38:47.000000000","message":"Why do we need module and maint_func options instead of one option?\n\nimport_class can import \u0027neutron.agent.linux.daemon.main\u0027 function and stevedore could perhaps import \u0027neutron.agent.linux.daemon:main\u0027.","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"5059b80774d38380c3772dc69a39f9afc1cfb5fa","unresolved":false,"context_lines":[{"line_number":267,"context_line":"                   help\u003d_(\u0027Where to look for a main function after \u0027"},{"line_number":268,"context_line":"                          \u0027daemonization. Example: \u0027"},{"line_number":269,"context_line":"                          \u0027neutron.agent.linux.daemon\u0027)),"},{"line_number":270,"context_line":"        cfg.StrOpt(\u0027main_func\u0027,"},{"line_number":271,"context_line":"                   default\u003d\u0027main\u0027,"},{"line_number":272,"context_line":"                   help\u003d_(\u0027Name of the main function within the module.\u0027)),"},{"line_number":273,"context_line":"    ]"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_97006eac","line":270,"in_reply_to":"3a961159_0f3e30b8","updated":"2015-01-07 14:46:40.000000000","message":"The point of this change is primarily to provide flexibility for testing purposes.  What are the advantages of using a more complex mechanism like stevedore for that, and would it be flexible enough to allow the use cases John intends?","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"97be7bd55e83f9b85361cd0412c90e16d1e3b967","unresolved":false,"context_lines":[{"line_number":281,"context_line":"                  \u0027config_file\u0027: cfg.CONF.config_file})"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"        module \u003d importutils.import_module(cfg.CONF.module)"},{"line_number":284,"context_line":"        func \u003d getattr(module, cfg.CONF.main_func)"},{"line_number":285,"context_line":"        func()"},{"line_number":286,"context_line":""},{"line_number":287,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_cfa6e80e","line":284,"updated":"2015-01-07 10:38:47.000000000","message":"don\u0027t know really why but i would prefer to compute func in __init__ and run it in run().","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"5059b80774d38380c3772dc69a39f9afc1cfb5fa","unresolved":false,"context_lines":[{"line_number":281,"context_line":"                  \u0027config_file\u0027: cfg.CONF.config_file})"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"        module \u003d importutils.import_module(cfg.CONF.module)"},{"line_number":284,"context_line":"        func \u003d getattr(module, cfg.CONF.main_func)"},{"line_number":285,"context_line":"        func()"},{"line_number":286,"context_line":""},{"line_number":287,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_f7be12d3","line":284,"in_reply_to":"3a961159_cfa6e80e","updated":"2015-01-07 14:46:40.000000000","message":"I think init should \u0027just work\u0027, and that doing potentially error-prone configuration (no guarantee these calls are going to succeed) is best done in a subsequent method call.","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"97be7bd55e83f9b85361cd0412c90e16d1e3b967","unresolved":false,"context_lines":[{"line_number":286,"context_line":""},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"def main():"},{"line_number":289,"context_line":"    ProcessDaemon.main()"},{"line_number":290,"context_line":""},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_8f3ce094","line":289,"updated":"2015-01-07 10:38:47.000000000","message":"the use of stevedore (see previous comment) do not require to define main function as stevedore is able to load \u0027neutron.agent.linux.daemon:ProcessDaemon.main\u0027","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"3ea394f419c0623695e24841c7a9c0d40fc73248","unresolved":false,"context_lines":[{"line_number":286,"context_line":""},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"def main():"},{"line_number":289,"context_line":"    ProcessDaemon.main()"},{"line_number":290,"context_line":""},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_a315173e","line":289,"in_reply_to":"3a961159_8f3ce094","updated":"2015-01-08 07:43:00.000000000","message":"I\u0027ve tried making stevedore work for me but didn\u0027t really manage to get it running. I think I\u0027ll let you and Maru hash whether stevedore is needed here (IMO it only adds another level of complication) before I make changes to this code.","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"09a24777786e6148111b156e10ba7717bd7ff821","unresolved":false,"context_lines":[{"line_number":274,"context_line":"    ]"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    def daemonize(self):"},{"line_number":277,"context_line":"        #NOTE: If the code imported by this class imports additional modules"},{"line_number":278,"context_line":"        # during runtime (for example, using __import__()), the code might fail"},{"line_number":279,"context_line":"        # since daemonize() changes the current working directory of the"},{"line_number":280,"context_line":"        # process, which interferes with how python\u0027s import works. Long story"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_a2bf18aa","line":277,"updated":"2015-01-14 23:04:47.000000000","message":"I\u0027m afraid I don\u0027t understand the rationale behind this addition despite this comment, and I think it represents an undesired change in the forking behavior which explicitly disavows any knowledge of the originating parent\u0027s working directory.  Why is the python path not inherited and/or sufficient for the child to be able to load modules?","commit_id":"a674e7ed9bfbe39c5adcc177411380b8d2e7a255"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4ac083e26414f80c900bba8721c6badb698beca8","unresolved":false,"context_lines":[{"line_number":274,"context_line":"    ]"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    def daemonize(self):"},{"line_number":277,"context_line":"        #NOTE: If the code imported by this class imports additional modules"},{"line_number":278,"context_line":"        # during runtime (for example, using __import__()), the code might fail"},{"line_number":279,"context_line":"        # since daemonize() changes the current working directory of the"},{"line_number":280,"context_line":"        # process, which interferes with how python\u0027s import works. Long story"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_00115df7","line":277,"in_reply_to":"3a961159_a2bf18aa","updated":"2015-01-19 07:57:07.000000000","message":"The story behind this is that if we ran the code in the neutron folder (/opt/openstack/neutron), the imports of other neutron code is done relatively. Then, once we change the cwd to something else (\"/\", which is the desired procedure for daemonizing), python looks for other neutron modules at the same place as before (so \"import neutron.server\" will now look for neutron at the new cwd (\"/\"). Obviously it can\u0027t find it.\n\n In [1]: import neutron\n In [2]: neutron\n Out[2]: \u003cmodule \u0027neutron\u0027 from \u0027neutron/__init__.pyc\u0027\u003e\n In [3]: import os\n In [4]: os.chdir(\"/\")\n In [5]: neutron.server\n ---------------------------------------------------------------------------\n AttributeError                            Traceback (most recent call last)\n \u003cipython-input-5-bfde0d0d71cb\u003e in \u003cmodule\u003e()\n ----\u003e 1 neutron.server\n AttributeError: \u0027module\u0027 object has no attribute \u0027server\u0027\n\nThis is how I understand the issue and sadly the only \"non-hackish\" solution I see is rewriting a big part of this patch to use the \"exec\" system call (running \"exec\" will clean the data segments and previous imports will be forgotten). If possible, I\u0027d like to push this with the ugly workaround as it and fix this code later down the line (beginning of March hopefully), so as to not postpone work on the spec (got some finals in the coming month so won\u0027t have much time to work on this).","commit_id":"a674e7ed9bfbe39c5adcc177411380b8d2e7a255"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d0ea113f32e84b273b18dccd5d2c7f247bdf35f0","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        cfg.StrOpt(\u0027pid_file\u0027, required\u003dTrue,"},{"line_number":145,"context_line":"                   help\u003d_(\u0027Path of the pidfile of the daemonized process.\u0027)),"},{"line_number":146,"context_line":"        cfg.BoolOpt(\u0027daemonize\u0027,"},{"line_number":147,"context_line":"                    default\u003dTrue,"},{"line_number":148,"context_line":"                    help\u003d_(\u0027Should run as daemon?\u0027)),"},{"line_number":149,"context_line":"    ]"},{"line_number":150,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"fa81d914_ac0a071f","line":147,"updated":"2015-01-29 00:23:13.000000000","message":"an option daemonize that can be disabled, and that controls the behavior of a class that is supposed to be a Daemon sounds really wrong. Can you elaborate as why this is needed?","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"bb1db0d0e005f4bb3f7c6a99ea3469a290c8c000","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        cfg.StrOpt(\u0027pid_file\u0027, required\u003dTrue,"},{"line_number":145,"context_line":"                   help\u003d_(\u0027Path of the pidfile of the daemonized process.\u0027)),"},{"line_number":146,"context_line":"        cfg.BoolOpt(\u0027daemonize\u0027,"},{"line_number":147,"context_line":"                    default\u003dTrue,"},{"line_number":148,"context_line":"                    help\u003d_(\u0027Should run as daemon?\u0027)),"},{"line_number":149,"context_line":"    ]"},{"line_number":150,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"da86d52c_3541095b","line":147,"in_reply_to":"fa81d914_ac0a071f","updated":"2015-02-02 16:45:08.000000000","message":"This was there in the original version, and didn\u0027t want to change the interface too much.","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d0ea113f32e84b273b18dccd5d2c7f247bdf35f0","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    def delete_pid(self):"},{"line_number":202,"context_line":"        try:"},{"line_number":203,"context_line":"            os.remove(str(self.pidfile))"},{"line_number":204,"context_line":"        except OSError:"},{"line_number":205,"context_line":"            pass"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def handle_sigterm(self, signum, frame):"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa81d914_a1369050","line":204,"updated":"2015-01-29 00:23:13.000000000","message":"This exception is pretty broad, we gotta be careful at ignoring blindly without even providing some debug information as to why the operation failed.","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d0ea113f32e84b273b18dccd5d2c7f247bdf35f0","unresolved":false,"context_lines":[{"line_number":207,"context_line":"    def handle_sigterm(self, signum, frame):"},{"line_number":208,"context_line":"        LOG.info(_LI(\"Received SIGTERM! Exiting!\"))"},{"line_number":209,"context_line":"        self.pidfile.unlock()"},{"line_number":210,"context_line":"        self.delete_pid()"},{"line_number":211,"context_line":"        sys.exit(0)"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    def start(self):"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa81d914_418f94cc","line":210,"updated":"2015-01-29 00:23:13.000000000","message":"isn\u0027t this taken care of by atexit.register that\u0027s run in daemonize?","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d0ea113f32e84b273b18dccd5d2c7f247bdf35f0","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        drop_privileges(self.user, self.group)"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    @classmethod"},{"line_number":235,"context_line":"    def configure(cls):"},{"line_number":236,"context_line":"        cfg.CONF.register_cli_opts(cls.OPTS)"},{"line_number":237,"context_line":"        cfg.CONF.register_cli_opts(cls.EXTRA_OPTS)"},{"line_number":238,"context_line":"        cfg.CONF(project\u003d\u0027neutron\u0027, default_config_files\u003d[])"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa81d914_01859cac","line":235,"updated":"2015-01-29 00:23:13.000000000","message":"why are these defined here rather than in the ProcessDaemon class?","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"bb1db0d0e005f4bb3f7c6a99ea3469a290c8c000","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        drop_privileges(self.user, self.group)"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"    @classmethod"},{"line_number":235,"context_line":"    def configure(cls):"},{"line_number":236,"context_line":"        cfg.CONF.register_cli_opts(cls.OPTS)"},{"line_number":237,"context_line":"        cfg.CONF.register_cli_opts(cls.EXTRA_OPTS)"},{"line_number":238,"context_line":"        cfg.CONF(project\u003d\u0027neutron\u0027, default_config_files\u003d[])"}],"source_content_type":"text/x-python","patch_set":17,"id":"da86d52c_300f5732","line":235,"in_reply_to":"fa81d914_01859cac","updated":"2015-02-02 16:45:08.000000000","message":"This is shared code with the metadata proxy, so it\u0027s here to avoid code duplication","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d0ea113f32e84b273b18dccd5d2c7f247bdf35f0","unresolved":false,"context_lines":[{"line_number":240,"context_line":"        utils.log_opt_values(LOG)"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    @classmethod"},{"line_number":243,"context_line":"    def main(cls):"},{"line_number":244,"context_line":"        cls.configure()"},{"line_number":245,"context_line":"        daemon \u003d cls(cfg.CONF.pid_file)"},{"line_number":246,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"fa81d914_ac42070e","line":243,"updated":"2015-01-29 00:23:13.000000000","message":"is this still needed if we dropped the use neutron-daemon command?","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"bb1db0d0e005f4bb3f7c6a99ea3469a290c8c000","unresolved":false,"context_lines":[{"line_number":240,"context_line":"        utils.log_opt_values(LOG)"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    @classmethod"},{"line_number":243,"context_line":"    def main(cls):"},{"line_number":244,"context_line":"        cls.configure()"},{"line_number":245,"context_line":"        daemon \u003d cls(cfg.CONF.pid_file)"},{"line_number":246,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"da86d52c_901d6369","line":243,"in_reply_to":"fa81d914_ac42070e","updated":"2015-02-02 16:45:08.000000000","message":"same as above","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d0ea113f32e84b273b18dccd5d2c7f247bdf35f0","unresolved":false,"context_lines":[{"line_number":242,"context_line":"    @classmethod"},{"line_number":243,"context_line":"    def main(cls):"},{"line_number":244,"context_line":"        cls.configure()"},{"line_number":245,"context_line":"        daemon \u003d cls(cfg.CONF.pid_file)"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"        if cfg.CONF.daemonize:"},{"line_number":248,"context_line":"            daemon.start()"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa81d914_ace92782","line":245,"updated":"2015-01-29 00:23:13.000000000","message":"this is not very clean. I can see that you moved most of the logic out of the namespace_proxy into the base class, but I\u0027d rather keep it the way it was, as it was a lot cleaner. What is the rationale for moving things around?","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d0ea113f32e84b273b18dccd5d2c7f247bdf35f0","unresolved":false,"context_lines":[{"line_number":284,"context_line":"        # daemonization happened."},{"line_number":285,"context_line":"        original_dir \u003d os.getcwd()"},{"line_number":286,"context_line":"        super(ProcessDaemon, self).daemonize()"},{"line_number":287,"context_line":"        os.chdir(original_dir)"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    def run(self):"},{"line_number":290,"context_line":"        super(ProcessDaemon, self).run()"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa81d914_c78714d6","line":287,"updated":"2015-01-29 00:23:13.000000000","message":"This is wrong: you\u0027re not supposed to change dir back to the original_dir, this violates how processes get daemonized.","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"bb1db0d0e005f4bb3f7c6a99ea3469a290c8c000","unresolved":false,"context_lines":[{"line_number":284,"context_line":"        # daemonization happened."},{"line_number":285,"context_line":"        original_dir \u003d os.getcwd()"},{"line_number":286,"context_line":"        super(ProcessDaemon, self).daemonize()"},{"line_number":287,"context_line":"        os.chdir(original_dir)"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"    def run(self):"},{"line_number":290,"context_line":"        super(ProcessDaemon, self).run()"}],"source_content_type":"text/x-python","patch_set":17,"id":"da86d52c_503a0bce","line":287,"in_reply_to":"fa81d914_c78714d6","updated":"2015-02-02 16:45:08.000000000","message":"Yep. It was a workaround gone bad.","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d0ea113f32e84b273b18dccd5d2c7f247bdf35f0","unresolved":false,"context_lines":[{"line_number":303,"context_line":"    ProcessDaemon.main()"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"},{"line_number":307,"context_line":"    main()"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa81d914_011cbc12","line":306,"updated":"2015-01-29 00:23:13.000000000","message":"is this still needed?","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"bb1db0d0e005f4bb3f7c6a99ea3469a290c8c000","unresolved":false,"context_lines":[{"line_number":303,"context_line":"    ProcessDaemon.main()"},{"line_number":304,"context_line":""},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"},{"line_number":307,"context_line":"    main()"}],"source_content_type":"text/x-python","patch_set":17,"id":"da86d52c_903483df","line":306,"in_reply_to":"fa81d914_011cbc12","updated":"2015-02-02 16:45:08.000000000","message":"Probably not.","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"}],"neutron/agent/metadata/namespace_proxy.py":[{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"97be7bd55e83f9b85361cd0412c90e16d1e3b967","unresolved":false,"context_lines":[{"line_number":170,"context_line":""},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"def main():"},{"line_number":173,"context_line":"    ProxyDaemon.main()"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_8f0a20d0","line":173,"updated":"2015-01-07 10:38:47.000000000","message":"same remark","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"}],"neutron/tests/common/helpers.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"09a24777786e6148111b156e10ba7717bd7ff821","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"@contextlib.contextmanager"},{"line_number":21,"context_line":"def create_tempdir(suffix\u003d\u0027\u0027, prefix\u003d\u0027\u0027, path\u003d\u0027/tmp\u0027):"},{"line_number":22,"context_line":"    \"\"\"Create a temporary directory usable in a \u0027with\u0027 statement.\"\"\""},{"line_number":23,"context_line":"    temp_path \u003d tempfile.mkdtemp(suffix, prefix, path)"},{"line_number":24,"context_line":"    yield temp_path"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_82505c5f","line":21,"updated":"2015-01-14 23:04:47.000000000","message":"Doh!  As nice as a context-based tempdir, it probably makes sense to use a fixture that cleans up after itself automatically instead, e.g.\n\nhttps://github.com/openstack/neutron/blob/master/neutron/tests/base.py#L88","commit_id":"a674e7ed9bfbe39c5adcc177411380b8d2e7a255"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4ac083e26414f80c900bba8721c6badb698beca8","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"@contextlib.contextmanager"},{"line_number":21,"context_line":"def create_tempdir(suffix\u003d\u0027\u0027, prefix\u003d\u0027\u0027, path\u003d\u0027/tmp\u0027):"},{"line_number":22,"context_line":"    \"\"\"Create a temporary directory usable in a \u0027with\u0027 statement.\"\"\""},{"line_number":23,"context_line":"    temp_path \u003d tempfile.mkdtemp(suffix, prefix, path)"},{"line_number":24,"context_line":"    yield temp_path"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_c0d595b7","line":21,"in_reply_to":"3a961159_82505c5f","updated":"2015-01-19 07:57:07.000000000","message":"Done","commit_id":"a674e7ed9bfbe39c5adcc177411380b8d2e7a255"}],"neutron/tests/functional/agent/linux/simple_daemon.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f9fa1121f28d4f42d39c1f4df66f74257e9a5877","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import eventlet"},{"line_number":16,"context_line":"eventlet.monkey_patch()"},{"line_number":17,"context_line":"from oslo.config import cfg"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from neutron.agent.linux import daemon"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_710fa339","line":16,"updated":"2014-12-21 04:34:48.000000000","message":"Is this call necessary here?  The daemon import would seem to ensure the same result.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4f29c1055b06ef38553558a1eee0b3c3e289dd91","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import eventlet"},{"line_number":16,"context_line":"eventlet.monkey_patch()"},{"line_number":17,"context_line":"from oslo.config import cfg"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from neutron.agent.linux import daemon"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_3b8be31f","line":16,"in_reply_to":"3a961159_710fa339","updated":"2014-12-25 09:13:11.000000000","message":"Done","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"97be7bd55e83f9b85361cd0412c90e16d1e3b967","unresolved":false,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"def main():"},{"line_number":41,"context_line":"    SimpleDaemon.main()"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"if __name__ \u003d\u003d \"__main__\":"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_cf10a802","line":41,"updated":"2015-01-07 10:38:47.000000000","message":"... :)","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"}],"neutron/tests/functional/agent/linux/test_daemon.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f9fa1121f28d4f42d39c1f4df66f74257e9a5877","unresolved":false,"context_lines":[{"line_number":18,"context_line":"import tempfile"},{"line_number":19,"context_line":"import time"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"import eventlet"},{"line_number":22,"context_line":"eventlet.monkey_patch()"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from neutron.agent.linux import utils"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_2c75c497","line":21,"updated":"2014-12-21 04:34:48.000000000","message":"Why is this import and subsequent monkeypatch required?  I don\u0027t see any eventlet-requiring code in this module.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4f29c1055b06ef38553558a1eee0b3c3e289dd91","unresolved":false,"context_lines":[{"line_number":18,"context_line":"import tempfile"},{"line_number":19,"context_line":"import time"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"import eventlet"},{"line_number":22,"context_line":"eventlet.monkey_patch()"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from neutron.agent.linux import utils"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_db73bf4c","line":21,"in_reply_to":"3a961159_2c75c497","updated":"2014-12-25 09:13:11.000000000","message":"I was under the (false) expression every executable file (or test file) should monkey_patch. Removing.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f9fa1121f28d4f42d39c1f4df66f74257e9a5877","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from neutron.tests import base"},{"line_number":26,"context_line":"from neutron.tests.functional.agent.linux import helpers"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"DAEMON_TEST_FILE \u003d \u0027/tmp/daemon_test\u0027"},{"line_number":29,"context_line":"DAEMON_TEST_OUTPUT \u003d \u0027There is no cake! The cake is a lie!\u0027"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_cc37e0e1","line":28,"updated":"2014-12-21 04:34:48.000000000","message":"Statically defining file targets for testing should be avoided.  Please generate the target filename with tempfile and provide it to the function via a conf option.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4f29c1055b06ef38553558a1eee0b3c3e289dd91","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from neutron.tests import base"},{"line_number":26,"context_line":"from neutron.tests.functional.agent.linux import helpers"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"DAEMON_TEST_FILE \u003d \u0027/tmp/daemon_test\u0027"},{"line_number":29,"context_line":"DAEMON_TEST_OUTPUT \u003d \u0027There is no cake! The cake is a lie!\u0027"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_7ba2cb4c","line":28,"in_reply_to":"3a961159_cc37e0e1","updated":"2014-12-25 09:13:11.000000000","message":"Done","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f9fa1121f28d4f42d39c1f4df66f74257e9a5877","unresolved":false,"context_lines":[{"line_number":63,"context_line":"            os.chdir(original_dir)"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def test_daemonized_process(self):"},{"line_number":66,"context_line":"        pid_file \u003d tempfile.NamedTemporaryFile(dir\u003d\u0027/tmp/\u0027)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        # Change directories to a different (neutral) dir. The reason is"},{"line_number":69,"context_line":"        # that later in the test, we run a python command and try to import"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_cc10c050","line":66,"updated":"2014-12-21 04:34:48.000000000","message":"(No action required) It is generally not required to provide the \u0027dir\u0027 parameter to  tempfile - a reasonable default will be used that can be overridden by environment variables.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4f29c1055b06ef38553558a1eee0b3c3e289dd91","unresolved":false,"context_lines":[{"line_number":63,"context_line":"            os.chdir(original_dir)"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def test_daemonized_process(self):"},{"line_number":66,"context_line":"        pid_file \u003d tempfile.NamedTemporaryFile(dir\u003d\u0027/tmp/\u0027)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        # Change directories to a different (neutral) dir. The reason is"},{"line_number":69,"context_line":"        # that later in the test, we run a python command and try to import"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_bb8353a8","line":66,"in_reply_to":"3a961159_cc10c050","updated":"2014-12-25 09:13:11.000000000","message":"Done","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f9fa1121f28d4f42d39c1f4df66f74257e9a5877","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        # a folder/file called neutron (for example, if we\u0027re in"},{"line_number":72,"context_line":"        # /opt/openstack/neutron), this will fail for PYTHONPATH reasons."},{"line_number":73,"context_line":"        #TODO(jschwarz): Find out why this is happening :\u003c"},{"line_number":74,"context_line":"        with self._change_dirs(\u0027/\u0027):"},{"line_number":75,"context_line":"            if os.path.exists(DAEMON_TEST_FILE):"},{"line_number":76,"context_line":"                os.unlink(DAEMON_TEST_FILE)"},{"line_number":77,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_6c1d0c62","line":74,"updated":"2014-12-21 04:34:48.000000000","message":"Ugh, definitely need to figure out how to remove this hackiness.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f9fa1121f28d4f42d39c1f4df66f74257e9a5877","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # /opt/openstack/neutron), this will fail for PYTHONPATH reasons."},{"line_number":73,"context_line":"        #TODO(jschwarz): Find out why this is happening :\u003c"},{"line_number":74,"context_line":"        with self._change_dirs(\u0027/\u0027):"},{"line_number":75,"context_line":"            if os.path.exists(DAEMON_TEST_FILE):"},{"line_number":76,"context_line":"                os.unlink(DAEMON_TEST_FILE)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"            self._execute_process(\u0027_test_daemonized\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_8ca7980a","line":75,"updated":"2014-12-21 04:34:48.000000000","message":"Even if there was a good reason to statically define the target filename, addCleanup would be preferable to cleaning up artifacts from a previous test run in any given test.  Every test run should do its best to leave the system in the state it found it.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4f29c1055b06ef38553558a1eee0b3c3e289dd91","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # /opt/openstack/neutron), this will fail for PYTHONPATH reasons."},{"line_number":73,"context_line":"        #TODO(jschwarz): Find out why this is happening :\u003c"},{"line_number":74,"context_line":"        with self._change_dirs(\u0027/\u0027):"},{"line_number":75,"context_line":"            if os.path.exists(DAEMON_TEST_FILE):"},{"line_number":76,"context_line":"                os.unlink(DAEMON_TEST_FILE)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"            self._execute_process(\u0027_test_daemonized\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_3bbf6362","line":75,"in_reply_to":"3a961159_8ca7980a","updated":"2014-12-25 09:13:11.000000000","message":"You\u0027re right. Changed the code to use a with statement to clean up the tempfile instead.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f9fa1121f28d4f42d39c1f4df66f74257e9a5877","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"            self.assertTrue(os.path.exists(pid_file.name))"},{"line_number":82,"context_line":"            helpers.wait_until_true(lambda: os.path.exists(DAEMON_TEST_FILE))"},{"line_number":83,"context_line":"            with open(DAEMON_TEST_FILE, \u0027r\u0027) as f:"},{"line_number":84,"context_line":"                self.assertEqual(DAEMON_TEST_OUTPUT, f.read())"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"            helpers.wait_until_true(lambda: not os.path.exists(pid_file.name))"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_4c05b00f","line":83,"updated":"2014-12-21 04:34:48.000000000","message":"(No action required) \u0027r\u0027 being the default, it can safely be omitted.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4f29c1055b06ef38553558a1eee0b3c3e289dd91","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"            self.assertTrue(os.path.exists(pid_file.name))"},{"line_number":82,"context_line":"            helpers.wait_until_true(lambda: os.path.exists(DAEMON_TEST_FILE))"},{"line_number":83,"context_line":"            with open(DAEMON_TEST_FILE, \u0027r\u0027) as f:"},{"line_number":84,"context_line":"                self.assertEqual(DAEMON_TEST_OUTPUT, f.read())"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"            helpers.wait_until_true(lambda: not os.path.exists(pid_file.name))"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_bbaa7318","line":83,"in_reply_to":"3a961159_4c05b00f","updated":"2014-12-25 09:13:11.000000000","message":"I\u0027m old fashioned, prefer to keep the \u0027r\u0027 here.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"290064e6e14c7a926484fdc4d2eb478eaa321332","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from neutron.tests import base"},{"line_number":26,"context_line":"from neutron.tests.functional.agent.linux import helpers"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"DAEMON_TEST_OUTPUT \u003d \u0027There is no cake! The cake is a lie!\u0027"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def _test_daemonized():"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_5cecf78f","line":28,"updated":"2014-12-30 07:43:45.000000000","message":"??","commit_id":"2f7a0c2c13b6515e5927eebf9e1493a4ba25f1a6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"7b04bd9090163a6b62fc6ac45077958c0ff9cc48","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from neutron.tests import base"},{"line_number":26,"context_line":"from neutron.tests.functional.agent.linux import helpers"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"DAEMON_TEST_OUTPUT \u003d \u0027There is no cake! The cake is a lie!\u0027"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def _test_daemonized():"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_b775f4fb","line":28,"in_reply_to":"3a961159_5cecf78f","updated":"2014-12-30 09:09:55.000000000","message":"It is a reference to Portal :)","commit_id":"2f7a0c2c13b6515e5927eebf9e1493a4ba25f1a6"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"290064e6e14c7a926484fdc4d2eb478eaa321332","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        output \u003d self._execute_process(\u0027_test_nondaemonized\u0027,"},{"line_number":110,"context_line":"                                       pid_file\u003dpid_file, daemonize\u003dFalse)"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"        self.assertEqual(output, DAEMON_TEST_OUTPUT)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def test_bad_args(self):"},{"line_number":115,"context_line":"        args \u003d [\u0027python\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_d72fe086","line":112,"updated":"2014-12-30 07:43:45.000000000","message":"expected value should be first","commit_id":"2f7a0c2c13b6515e5927eebf9e1493a4ba25f1a6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"7b04bd9090163a6b62fc6ac45077958c0ff9cc48","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        output \u003d self._execute_process(\u0027_test_nondaemonized\u0027,"},{"line_number":110,"context_line":"                                       pid_file\u003dpid_file, daemonize\u003dFalse)"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"        self.assertEqual(output, DAEMON_TEST_OUTPUT)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def test_bad_args(self):"},{"line_number":115,"context_line":"        args \u003d [\u0027python\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_f75e9c78","line":112,"in_reply_to":"3a961159_d72fe086","updated":"2014-12-30 09:09:55.000000000","message":"Done","commit_id":"2f7a0c2c13b6515e5927eebf9e1493a4ba25f1a6"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"118750fbd26a675808cc15b3b24486f6c72fa296","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    def test_daemonized_process(self):"},{"line_number":80,"context_line":"        pid_file \u003d tempfile.NamedTemporaryFile()"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"        # Change directories to a different (neutral) dir. The reason is"},{"line_number":83,"context_line":"        # that later in the test, we run a python command and try to import"},{"line_number":84,"context_line":"        # \u0027neutron.tests.functional...\u0027. In case our current working dir has"},{"line_number":85,"context_line":"        # a folder/file called neutron (for example, if we\u0027re in"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_aeecbaf4","line":82,"updated":"2014-12-30 14:41:46.000000000","message":"I run the test on my local /opt/openstack/neutron commenting the code that changes the directory and it seem to work. Can you describe what kind of error you are getting?","commit_id":"12739236e5b1c70bf7a66801bbc3325891d0a6c9"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"c4ff8cbd4c3de294afba4972b544a8ad3fbb979d","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    def test_daemonized_process(self):"},{"line_number":80,"context_line":"        pid_file \u003d tempfile.NamedTemporaryFile()"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"        # Change directories to a different (neutral) dir. The reason is"},{"line_number":83,"context_line":"        # that later in the test, we run a python command and try to import"},{"line_number":84,"context_line":"        # \u0027neutron.tests.functional...\u0027. In case our current working dir has"},{"line_number":85,"context_line":"        # a folder/file called neutron (for example, if we\u0027re in"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_0399f26d","line":82,"in_reply_to":"3a961159_aeecbaf4","updated":"2015-01-07 08:59:16.000000000","message":"How weird. Testing this now seems to work... I remember clearly having an ImportError since it was trying to import the wrong module.\n\nI removed this code so hopefully it\u0027s fixed now.","commit_id":"12739236e5b1c70bf7a66801bbc3325891d0a6c9"},{"author":{"_account_id":6788,"name":"Rossella Sblendido","email":"rsblendido@suse.com","username":"rossella-o"},"change_message_id":"0f77542075f27ac2b58943d065e319fb37eed473","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                    timeout\u003d120)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def test_nondaemonized_process(self):"},{"line_number":108,"context_line":"        pid_file \u003d tempfile.NamedTemporaryFile(dir\u003d\u0027/tmp/\u0027)"},{"line_number":109,"context_line":"        output \u003d self._execute_process(\u0027_test_nondaemonized\u0027,"},{"line_number":110,"context_line":"                                       pid_file\u003dpid_file, daemonize\u003dFalse)"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_2a00b301","line":108,"updated":"2014-12-30 16:51:04.000000000","message":"you might want to remove the directory to be consistent with line 80","commit_id":"12739236e5b1c70bf7a66801bbc3325891d0a6c9"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"c4ff8cbd4c3de294afba4972b544a8ad3fbb979d","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                    timeout\u003d120)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def test_nondaemonized_process(self):"},{"line_number":108,"context_line":"        pid_file \u003d tempfile.NamedTemporaryFile(dir\u003d\u0027/tmp/\u0027)"},{"line_number":109,"context_line":"        output \u003d self._execute_process(\u0027_test_nondaemonized\u0027,"},{"line_number":110,"context_line":"                                       pid_file\u003dpid_file, daemonize\u003dFalse)"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_c3a0caa7","line":108,"in_reply_to":"3a961159_2a00b301","updated":"2015-01-07 08:59:16.000000000","message":"Done","commit_id":"12739236e5b1c70bf7a66801bbc3325891d0a6c9"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"fa5a4a8258bfc47977a366354f11719020c9b62b","unresolved":false,"context_lines":[{"line_number":31,"context_line":"def _test_daemonized():"},{"line_number":32,"context_line":"    with open(cfg.CONF.test_file, \u0027w\u0027) as f:"},{"line_number":33,"context_line":"        f.write(DAEMON_TEST_OUTPUT)"},{"line_number":34,"context_line":"    time.sleep(10)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def _test_nondaemonized():"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_187cbf1f","line":34,"updated":"2015-01-07 02:27:00.000000000","message":"Why is this sleep required?","commit_id":"7509e1f4f64e05d453b2d577e728ec2cb46fe9d6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"65bc3f2a02936a680a29321d4caad915448091c7","unresolved":false,"context_lines":[{"line_number":31,"context_line":"def _test_daemonized():"},{"line_number":32,"context_line":"    with open(cfg.CONF.test_file, \u0027w\u0027) as f:"},{"line_number":33,"context_line":"        f.write(DAEMON_TEST_OUTPUT)"},{"line_number":34,"context_line":"    time.sleep(10)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def _test_nondaemonized():"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_f901b239","line":34,"in_reply_to":"3a961159_187cbf1f","updated":"2015-01-07 09:40:01.000000000","message":"This is required because of the check in L89. Once the process is finished, the pidfile is removed, so instead I keep the process up for a minimal amount of time and let the test capture the pidfile instead. Added some comments to indicate this.","commit_id":"7509e1f4f64e05d453b2d577e728ec2cb46fe9d6"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"fa5a4a8258bfc47977a366354f11719020c9b62b","unresolved":false,"context_lines":[{"line_number":95,"context_line":""},{"line_number":96,"context_line":"            helpers.wait_until_true("},{"line_number":97,"context_line":"                lambda: not os.path.exists(pid_file.name),"},{"line_number":98,"context_line":"                timeout\u003d120)"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def test_nondaemonized_process(self):"},{"line_number":101,"context_line":"        pid_file \u003d tempfile.NamedTemporaryFile()"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_5872c74a","line":98,"updated":"2015-01-07 02:27:00.000000000","message":"This timeout should probably be removed, since tox.ini for the functional jobs sets a timeout of 90s.","commit_id":"7509e1f4f64e05d453b2d577e728ec2cb46fe9d6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"65bc3f2a02936a680a29321d4caad915448091c7","unresolved":false,"context_lines":[{"line_number":95,"context_line":""},{"line_number":96,"context_line":"            helpers.wait_until_true("},{"line_number":97,"context_line":"                lambda: not os.path.exists(pid_file.name),"},{"line_number":98,"context_line":"                timeout\u003d120)"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"    def test_nondaemonized_process(self):"},{"line_number":101,"context_line":"        pid_file \u003d tempfile.NamedTemporaryFile()"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_59f5664f","line":98,"in_reply_to":"3a961159_5872c74a","updated":"2015-01-07 09:40:01.000000000","message":"Done","commit_id":"7509e1f4f64e05d453b2d577e728ec2cb46fe9d6"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"fa5a4a8258bfc47977a366354f11719020c9b62b","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        output \u003d self._execute_process(\u0027_test_nondaemonized\u0027,"},{"line_number":103,"context_line":"                                       pid_file\u003dpid_file, daemonize\u003dFalse)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        self.assertEqual(DAEMON_TEST_OUTPUT, output)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def test_bad_args(self):"},{"line_number":108,"context_line":"        args \u003d [\u0027python\u0027,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_b8e2cbd2","line":105,"updated":"2015-01-07 02:27:00.000000000","message":"(no change required) requiring specific ordering in assertEqual is the nitiests of nits.  I think we have more important things to care about in review, and for anyone that actually cares about this, this nova thread is probably something you actually enjoyed reading:\n\nhttp://lists.openstack.org/pipermail/openstack-dev/2014-November/051189.html","commit_id":"7509e1f4f64e05d453b2d577e728ec2cb46fe9d6"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"fa5a4a8258bfc47977a366354f11719020c9b62b","unresolved":false,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        self.assertEqual(DAEMON_TEST_OUTPUT, output)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def test_bad_args(self):"},{"line_number":108,"context_line":"        args \u003d [\u0027python\u0027,"},{"line_number":109,"context_line":"                \u0027-c\u0027, \u0027from neutron.agent.linux import daemon; daemon.main()\u0027,"},{"line_number":110,"context_line":"                \u0027--pid_file\u0027, tempfile.NamedTemporaryFile().name,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_18e75f13","line":107,"updated":"2015-01-07 02:27:00.000000000","message":"I don\u0027t think this test is useful once the options are required - it would only duplicate the configuration.","commit_id":"7509e1f4f64e05d453b2d577e728ec2cb46fe9d6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"65bc3f2a02936a680a29321d4caad915448091c7","unresolved":false,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        self.assertEqual(DAEMON_TEST_OUTPUT, output)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    def test_bad_args(self):"},{"line_number":108,"context_line":"        args \u003d [\u0027python\u0027,"},{"line_number":109,"context_line":"                \u0027-c\u0027, \u0027from neutron.agent.linux import daemon; daemon.main()\u0027,"},{"line_number":110,"context_line":"                \u0027--pid_file\u0027, tempfile.NamedTemporaryFile().name,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_594726a3","line":107,"in_reply_to":"3a961159_18e75f13","updated":"2015-01-07 09:40:01.000000000","message":"Done","commit_id":"7509e1f4f64e05d453b2d577e728ec2cb46fe9d6"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"97be7bd55e83f9b85361cd0412c90e16d1e3b967","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        return utils.execute(args)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    @contextlib.contextmanager"},{"line_number":74,"context_line":"    def _change_dirs(self, new_dir):"},{"line_number":75,"context_line":"        original_dir \u003d os.getcwd()"},{"line_number":76,"context_line":"        try:"},{"line_number":77,"context_line":"            os.chdir(new_dir)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_cfdee83d","line":74,"updated":"2015-01-07 10:38:47.000000000","message":"never used?","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"3ea394f419c0623695e24841c7a9c0d40fc73248","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        return utils.execute(args)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    @contextlib.contextmanager"},{"line_number":74,"context_line":"    def _change_dirs(self, new_dir):"},{"line_number":75,"context_line":"        original_dir \u003d os.getcwd()"},{"line_number":76,"context_line":"        try:"},{"line_number":77,"context_line":"            os.chdir(new_dir)"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_2db75886","line":74,"in_reply_to":"3a961159_cfdee83d","updated":"2015-01-08 07:43:00.000000000","message":"Never used. Deleted.","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"97be7bd55e83f9b85361cd0412c90e16d1e3b967","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            os.chdir(original_dir)"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def test_daemonized_process(self):"},{"line_number":83,"context_line":"        pid_file \u003d tempfile.NamedTemporaryFile()"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        with tempfile.NamedTemporaryFile() as test_file:"},{"line_number":86,"context_line":"            self._execute_process("}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_ef7ba426","line":83,"updated":"2015-01-07 10:38:47.000000000","message":"i have always seen tempfile.*TemporaryFile used as context:\n\n  with tempfile.NamedTemporaryFile(delete\u003dFalse) as pid_file:\n   ...","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"3ea394f419c0623695e24841c7a9c0d40fc73248","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            os.chdir(original_dir)"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def test_daemonized_process(self):"},{"line_number":83,"context_line":"        pid_file \u003d tempfile.NamedTemporaryFile()"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        with tempfile.NamedTemporaryFile() as test_file:"},{"line_number":86,"context_line":"            self._execute_process("}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_ad8b48be","line":83,"in_reply_to":"3a961159_5797e68f","updated":"2015-01-08 07:43:00.000000000","message":"I think adding a temp dir and putting a staticly named file in it will hurt readability, and using a context manager looks weird since the resource in question is deleted halfway through the process. I\u0027ll add a comment inline so it\u0027s clear how this is used.","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"5445a3fb9f1407549963e7cffcf755b81166319b","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            os.chdir(original_dir)"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def test_daemonized_process(self):"},{"line_number":83,"context_line":"        pid_file \u003d tempfile.NamedTemporaryFile()"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        with tempfile.NamedTemporaryFile() as test_file:"},{"line_number":86,"context_line":"            self._execute_process("}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_1e50f95c","line":83,"in_reply_to":"3a961159_ad8b48be","updated":"2015-01-08 14:11:20.000000000","message":"What about the case of the daemon not cleaning up after itself?  The pid file could potentially persist.  By using a temp directory as the target, there could be greater assurance that resources are cleaned up even in the face of failure (which the current approach does not guarantee).","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"5059b80774d38380c3772dc69a39f9afc1cfb5fa","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            os.chdir(original_dir)"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def test_daemonized_process(self):"},{"line_number":83,"context_line":"        pid_file \u003d tempfile.NamedTemporaryFile()"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        with tempfile.NamedTemporaryFile() as test_file:"},{"line_number":86,"context_line":"            self._execute_process("}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_5797e68f","line":83,"in_reply_to":"3a961159_ef7ba426","updated":"2015-01-07 14:46:40.000000000","message":"That would hold a reference to the file and complicate it\u0027s management by the daemon.  In this case, the point is simply to get an unused filename.  Maybe using a temporary directory as a context and using a statically filename would be preferable?","commit_id":"a459fe047a887b88e17ee15ed55bd081e51441a4"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d0ea113f32e84b273b18dccd5d2c7f247bdf35f0","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    # Once this function returns, the daemonize process exits and the pidfile"},{"line_number":35,"context_line":"    # is removed. We sleep for a minimal amount of time so that the test can"},{"line_number":36,"context_line":"    # actually check for the pidfile\u0027s existance."},{"line_number":37,"context_line":"    time.sleep(10)"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"def _test_nondaemonized():"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa81d914_7c051e79","line":37,"updated":"2015-01-29 00:23:13.000000000","message":"this is prone to races. There\u0027s no guarantee that you\u0027re able to get the timing right between here and when you check for file existence. Something may yield and prevent you from having the test pass reliably.","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d0ea113f32e84b273b18dccd5d2c7f247bdf35f0","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    # Once this function returns, the daemonize process exits and the pidfile"},{"line_number":35,"context_line":"    # is removed. We sleep for a minimal amount of time so that the test can"},{"line_number":36,"context_line":"    # actually check for the pidfile\u0027s existance."},{"line_number":37,"context_line":"    time.sleep(10)"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"def _test_nondaemonized():"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa81d914_bc36b650","line":37,"updated":"2015-01-29 00:23:13.000000000","message":"this is prone to races. There\u0027s no guarantee that you\u0027re able to get the timing right between here and when you check for file existence. Something may yield and prevent you from having the test pass reliably.","commit_id":"7a21190569ca3cd0371364d97b09d5e811f438ec"}],"neutron/tests/unit/test_metadata_namespace_proxy.py":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f9fa1121f28d4f42d39c1f4df66f74257e9a5877","unresolved":false,"context_lines":[{"line_number":274,"context_line":"    def test_init(self):"},{"line_number":275,"context_line":"        with mock.patch(\u0027neutron.agent.linux.daemon.Pidfile\u0027):"},{"line_number":276,"context_line":"            with mock.patch.object(ns_proxy, \u0027cfg\u0027) as cfg:"},{"line_number":277,"context_line":"                cfg.CONF.router_id \u003d \u0027router_id\u0027"},{"line_number":278,"context_line":"                cfg.CONF.network_id \u003d \u0027net_id\u0027"},{"line_number":279,"context_line":"                cfg.CONF.metadata_port \u003d 9697"},{"line_number":280,"context_line":"                cfg.CONF.pid_file \u003d \u0027pidfile\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_acc45492","line":277,"updated":"2014-12-21 04:34:48.000000000","message":"Setting the conf can probably be factored out (also appears in test_run).  \n\n(No action required) Did I mention how much I dislike global conf?","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4f29c1055b06ef38553558a1eee0b3c3e289dd91","unresolved":false,"context_lines":[{"line_number":274,"context_line":"    def test_init(self):"},{"line_number":275,"context_line":"        with mock.patch(\u0027neutron.agent.linux.daemon.Pidfile\u0027):"},{"line_number":276,"context_line":"            with mock.patch.object(ns_proxy, \u0027cfg\u0027) as cfg:"},{"line_number":277,"context_line":"                cfg.CONF.router_id \u003d \u0027router_id\u0027"},{"line_number":278,"context_line":"                cfg.CONF.network_id \u003d \u0027net_id\u0027"},{"line_number":279,"context_line":"                cfg.CONF.metadata_port \u003d 9697"},{"line_number":280,"context_line":"                cfg.CONF.pid_file \u003d \u0027pidfile\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_86a8fedc","line":277,"in_reply_to":"3a961159_acc45492","updated":"2014-12-25 09:13:11.000000000","message":"Actually, you didn\u0027t. Thank got it\u0027s marked (No action required), else I was about to replace it ;-)\n\nDone.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f9fa1121f28d4f42d39c1f4df66f74257e9a5877","unresolved":false,"context_lines":[{"line_number":276,"context_line":"            with mock.patch.object(ns_proxy, \u0027cfg\u0027) as cfg:"},{"line_number":277,"context_line":"                cfg.CONF.router_id \u003d \u0027router_id\u0027"},{"line_number":278,"context_line":"                cfg.CONF.network_id \u003d \u0027net_id\u0027"},{"line_number":279,"context_line":"                cfg.CONF.metadata_port \u003d 9697"},{"line_number":280,"context_line":"                cfg.CONF.pid_file \u003d \u0027pidfile\u0027"},{"line_number":281,"context_line":"                pd \u003d ns_proxy.ProxyDaemon(\u0027pidfile\u0027)"},{"line_number":282,"context_line":"                self.assertEqual(pd.router_id, \u0027router_id\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_6c3eec8c","line":279,"updated":"2014-12-21 04:34:48.000000000","message":"Given that there is a default value for metadata_port, does it have to be supplied.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4f29c1055b06ef38553558a1eee0b3c3e289dd91","unresolved":false,"context_lines":[{"line_number":276,"context_line":"            with mock.patch.object(ns_proxy, \u0027cfg\u0027) as cfg:"},{"line_number":277,"context_line":"                cfg.CONF.router_id \u003d \u0027router_id\u0027"},{"line_number":278,"context_line":"                cfg.CONF.network_id \u003d \u0027net_id\u0027"},{"line_number":279,"context_line":"                cfg.CONF.metadata_port \u003d 9697"},{"line_number":280,"context_line":"                cfg.CONF.pid_file \u003d \u0027pidfile\u0027"},{"line_number":281,"context_line":"                pd \u003d ns_proxy.ProxyDaemon(\u0027pidfile\u0027)"},{"line_number":282,"context_line":"                self.assertEqual(pd.router_id, \u0027router_id\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_06b40e34","line":279,"in_reply_to":"3a961159_6c3eec8c","updated":"2014-12-25 09:13:11.000000000","message":"Yes, since I\u0027m using a mocked oslo config (so the default isn\u0027t supplied after all).","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"}],"setup.cfg":[{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"f9fa1121f28d4f42d39c1f4df66f74257e9a5877","unresolved":false,"context_lines":[{"line_number":124,"context_line":"    neutron-ofagent-agent \u003d neutron.plugins.ofagent.agent.main:main"},{"line_number":125,"context_line":"    neutron-sriov-nic-agent \u003d neutron.plugins.sriovnicagent.sriov_nic_agent:main"},{"line_number":126,"context_line":"    neutron-sanity-check \u003d neutron.cmd.sanity_check:main"},{"line_number":127,"context_line":"    neutron-daemon \u003d neutron.agent.linux.daemon:main"},{"line_number":128,"context_line":"neutron.core_plugins \u003d"},{"line_number":129,"context_line":"    bigswitch \u003d neutron.plugins.bigswitch.plugin:NeutronRestProxyV2"},{"line_number":130,"context_line":"    brocade \u003d neutron.plugins.brocade.NeutronPlugin:BrocadePluginV2"}],"source_content_type":"text/x-ttcn-cfg","patch_set":9,"id":"3a961159_6c798c81","line":127,"updated":"2014-12-21 04:34:48.000000000","message":"Why does this need to be configured as an entry point?","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"4f29c1055b06ef38553558a1eee0b3c3e289dd91","unresolved":false,"context_lines":[{"line_number":124,"context_line":"    neutron-ofagent-agent \u003d neutron.plugins.ofagent.agent.main:main"},{"line_number":125,"context_line":"    neutron-sriov-nic-agent \u003d neutron.plugins.sriovnicagent.sriov_nic_agent:main"},{"line_number":126,"context_line":"    neutron-sanity-check \u003d neutron.cmd.sanity_check:main"},{"line_number":127,"context_line":"    neutron-daemon \u003d neutron.agent.linux.daemon:main"},{"line_number":128,"context_line":"neutron.core_plugins \u003d"},{"line_number":129,"context_line":"    bigswitch \u003d neutron.plugins.bigswitch.plugin:NeutronRestProxyV2"},{"line_number":130,"context_line":"    brocade \u003d neutron.plugins.brocade.NeutronPlugin:BrocadePluginV2"}],"source_content_type":"text/x-ttcn-cfg","patch_set":9,"id":"3a961159_46ec562a","line":127,"in_reply_to":"3a961159_6c798c81","updated":"2014-12-25 09:13:11.000000000","message":"Since I prefer keeping this in the non-test library, and since it\u0027s meant to be an executable (has a main() and accepts cmdline arguments), I figured it should also have an entry point.","commit_id":"dbdf02aa133af41c0b2e338fb7d7ff4259b94615"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"fa5a4a8258bfc47977a366354f11719020c9b62b","unresolved":false,"context_lines":[{"line_number":124,"context_line":"    neutron-ofagent-agent \u003d neutron.plugins.ofagent.agent.main:main"},{"line_number":125,"context_line":"    neutron-sriov-nic-agent \u003d neutron.plugins.sriovnicagent.sriov_nic_agent:main"},{"line_number":126,"context_line":"    neutron-sanity-check \u003d neutron.cmd.sanity_check:main"},{"line_number":127,"context_line":"    neutron-daemon \u003d neutron.agent.linux.daemon:main"},{"line_number":128,"context_line":"neutron.core_plugins \u003d"},{"line_number":129,"context_line":"    bigswitch \u003d neutron.plugins.bigswitch.plugin:NeutronRestProxyV2"},{"line_number":130,"context_line":"    brocade \u003d neutron.plugins.brocade.NeutronPlugin:BrocadePluginV2"}],"source_content_type":"text/x-ttcn-cfg","patch_set":12,"id":"3a961159_d5fa0076","line":127,"updated":"2015-01-07 02:27:00.000000000","message":"I don\u0027t think this is going to be usable outside of the tests, so an entry point is not suggested. Invoking a generic command that can run arbitrary code is not something that any sane person is going to want to grant sudo privileges to.","commit_id":"7509e1f4f64e05d453b2d577e728ec2cb46fe9d6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"65bc3f2a02936a680a29321d4caad915448091c7","unresolved":false,"context_lines":[{"line_number":124,"context_line":"    neutron-ofagent-agent \u003d neutron.plugins.ofagent.agent.main:main"},{"line_number":125,"context_line":"    neutron-sriov-nic-agent \u003d neutron.plugins.sriovnicagent.sriov_nic_agent:main"},{"line_number":126,"context_line":"    neutron-sanity-check \u003d neutron.cmd.sanity_check:main"},{"line_number":127,"context_line":"    neutron-daemon \u003d neutron.agent.linux.daemon:main"},{"line_number":128,"context_line":"neutron.core_plugins \u003d"},{"line_number":129,"context_line":"    bigswitch \u003d neutron.plugins.bigswitch.plugin:NeutronRestProxyV2"},{"line_number":130,"context_line":"    brocade \u003d neutron.plugins.brocade.NeutronPlugin:BrocadePluginV2"}],"source_content_type":"text/x-ttcn-cfg","patch_set":12,"id":"3a961159_f9c9b25a","line":127,"in_reply_to":"3a961159_d5fa0076","updated":"2015-01-07 09:40:01.000000000","message":"Done","commit_id":"7509e1f4f64e05d453b2d577e728ec2cb46fe9d6"}]}