)]}'
{"zuul/lib/keystorage.py":[{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"4e5501cf1945bf67582519fc141fd0fb585cf7d1","unresolved":true,"context_lines":[{"line_number":336,"context_line":"        self.kazoo_client.create(key_path, value\u003ddata, makepath\u003dTrue)"},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"    def getProjectSecretsKeys(self, connection_name, project_name):"},{"line_number":339,"context_line":"        key_project_name \u003d strings.unique_project_name(project_name)"},{"line_number":340,"context_line":"        key_path \u003d self.SECRETS_PATH.format(connection_name, key_project_name)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"b9afe217_09b024ae","line":339,"updated":"2021-04-19 08:11:17.000000000","message":"In other places where this could be a problem I\u0027ve used `urllib.parse.quote_plus` so far, e.g. for the semaphore name. Maybe we should decide on a common method for escaping. The zlib approach will probably fail in cases where there are chars other than \"/\" that we need to escape. IMHO `quote_plus()` is the better choice in this regard.","commit_id":"8eda1e908063a9dbfa598d68cff3e6d1ba7c1c47"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"8a368f91ad228e34a6b476507e3b49eb484d8fff","unresolved":false,"context_lines":[{"line_number":336,"context_line":"        self.kazoo_client.create(key_path, value\u003ddata, makepath\u003dTrue)"},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"    def getProjectSecretsKeys(self, connection_name, project_name):"},{"line_number":339,"context_line":"        key_project_name \u003d strings.unique_project_name(project_name)"},{"line_number":340,"context_line":"        key_path \u003d self.SECRETS_PATH.format(connection_name, key_project_name)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1be0be54_780683e6","line":339,"updated":"2021-04-19 16:18:24.000000000","message":"Let\u0027s move this conversation over to where the unique string is generated...","commit_id":"8eda1e908063a9dbfa598d68cff3e6d1ba7c1c47"}],"zuul/lib/strings.py":[{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"8a368f91ad228e34a6b476507e3b49eb484d8fff","unresolved":false,"context_lines":[{"line_number":22,"context_line":"    crc \u003d zlib.crc32(project_name.encode(\u0027utf8\u0027))"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    name \u003d TR_RE.sub(\u0027_\u0027, project_name)"},{"line_number":25,"context_line":"    return f\u0027{crc:02x}-{name}\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"38bd3b09_a5040cbd","line":25,"updated":"2021-04-19 16:18:24.000000000","message":"\u003e Simon Westphahl: In other places where this could be a problem I\u0027ve used `urllib.parse.quote_plus` so far, e.g. for the semaphore name. Maybe we should decide on a common method for escaping. The zlib approach will probably fail in cases where there are chars other than \"/\" that we need to escape. IMHO `quote_plus()` is the better choice in this regard.\n\nThis approach does two things: it translates / to _, and it adds a crc32 to virtually guarantee uniqueness (in the case that two names escape to the same value, ie foo/bar \u003d\u003d foo_bar).\n\nI don\u0027t see how the crc32 part of that (which comes from zlib) can fail if we add more escaped characters.\n\nWe could also use quote_plus instead, though I intend to use the same thing for filesystems, and we are guaranteed that \u0027/\u0027 will occur in project names, so almost every repo path on disk will look like gerrit/foo%20bar\n\nAs a bash user, I find that mildly annoying, but thanks to tab completion, not a show-stopper.  :)\n\nSo all things considered, I\u0027d be happy to switch to quote_plus for this.  I think it will achieve the main goal which is to flatten the repo hierarchy so that foo/bar is not under foo/.  AFAICT it poses no problems with uniqueness.  Compared to the crc32 method, it has the advantage of sorting, but a slight disadvantage in readability.","commit_id":"8eda1e908063a9dbfa598d68cff3e6d1ba7c1c47"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"4deb8857babbf15f44909acf61c4ba1297db4941","unresolved":false,"context_lines":[{"line_number":22,"context_line":"    crc \u003d zlib.crc32(project_name.encode(\u0027utf8\u0027))"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    name \u003d TR_RE.sub(\u0027_\u0027, project_name)"},{"line_number":25,"context_line":"    return f\u0027{crc:02x}-{name}\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"f03f197d_5a062852","line":25,"updated":"2021-04-20 16:50:50.000000000","message":"Makes sense.  My idea was we would expand the TR_RE as needed for other characters.","commit_id":"8eda1e908063a9dbfa598d68cff3e6d1ba7c1c47"},{"author":{"_account_id":27582,"name":"Simon Westphahl","email":"simon.westphahl@bmw.de","username":"simon.westphahl"},"change_message_id":"1079495351102eaffdf26226856e6f538d75dea8","unresolved":false,"context_lines":[{"line_number":22,"context_line":"    crc \u003d zlib.crc32(project_name.encode(\u0027utf8\u0027))"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"    name \u003d TR_RE.sub(\u0027_\u0027, project_name)"},{"line_number":25,"context_line":"    return f\u0027{crc:02x}-{name}\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"60bb8552_bade574f","line":25,"in_reply_to":"38bd3b09_a5040cbd","updated":"2021-04-20 05:34:18.000000000","message":"I agree that your approach is more readable. My main concern was other chars that are not allowed as part of the ZK path that we also need to escape (interestingly spaces seem to be fine): https://zookeeper.apache.org/doc/current/zookeeperProgrammers.html#ch_zkDataModel\n\n`quote_plus()` takes care of of most of those cases, except for the standalone \"./..\".\n\nI\u0027m also fine with extending this in a way that we can use this for other strings that need to be escaped.","commit_id":"8eda1e908063a9dbfa598d68cff3e6d1ba7c1c47"}]}
