New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 846935 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Feature

Blocking:
issue 832733



Sign in to add a comment

Implement discovery service for pRPC Python

Project Member Reported by no...@chromium.org, May 25 2018

Issue description

Implement discovery service for pRPC Python

It will enable RPC Explorer and prpc command tool for buildbucket API v2 which is currently in Python.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/bd85d6af3a4ae25aabaa18f172bb39a2662989c2

commit bd85d6af3a4ae25aabaa18f172bb39a2662989c2
Author: Nodir Turakulov <nodir@google.com>
Date: Wed May 30 22:22:06 2018

[prpc] Add discovery service

This will enable RPC Explorer for Buildbucket API V2.
Also prpc cmdline tool.

Bug:  846935 
Change-Id: I5876b72b5bfeb8f207dfb4a80f0ac06f8fb958b9
Reviewed-on: https://chromium-review.googlesource.com/1074197
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
Commit-Queue: Nodir Turakulov <nodir@chromium.org>

[add] https://crrev.com/bd85d6af3a4ae25aabaa18f172bb39a2662989c2/appengine/components/components/prpc/discovery/README.md
[add] https://crrev.com/bd85d6af3a4ae25aabaa18f172bb39a2662989c2/appengine/components/components/prpc/discovery/__init__.py
[add] https://crrev.com/bd85d6af3a4ae25aabaa18f172bb39a2662989c2/appengine/components/components/prpc/discovery/service.proto
[add] https://crrev.com/bd85d6af3a4ae25aabaa18f172bb39a2662989c2/appengine/components/components/prpc/discovery/service.py
[add] https://crrev.com/bd85d6af3a4ae25aabaa18f172bb39a2662989c2/appengine/components/components/prpc/discovery/service_pb2.py
[add] https://crrev.com/bd85d6af3a4ae25aabaa18f172bb39a2662989c2/appengine/components/components/prpc/discovery/service_prpc_pb2.py
[add] https://crrev.com/bd85d6af3a4ae25aabaa18f172bb39a2662989c2/appengine/components/components/prpc/discovery/service_test.py
[modify] https://crrev.com/bd85d6af3a4ae25aabaa18f172bb39a2662989c2/appengine/components/components/prpc/server.py
[modify] https://crrev.com/bd85d6af3a4ae25aabaa18f172bb39a2662989c2/appengine/components/components/prpc/test/test_prpc_pb2.py
[modify] https://crrev.com/bd85d6af3a4ae25aabaa18f172bb39a2662989c2/appengine/components/tools/protoc-gen-prpc-python

Project Member

Comment 2 by bugdroid1@chromium.org, May 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/6942a0b15aa9cc5c504a5df4ef5140a2947027db

commit 6942a0b15aa9cc5c504a5df4ef5140a2947027db
Author: Nodir Turakulov <nodir@google.com>
Date: Wed May 30 22:46:26 2018

[prpc] Add missing discovery/test

https://chromium-review.googlesource.com/c/infra/luci/luci-py/+/1074197
got landed with broken tests (some files were missing).
PRESUBMIT didn't fail.

Update presubmit to run prpc tests. Add the missing files.

Bug:  846935 
Change-Id: I4c52fbb6a4e8469cec1c11ede2585fa323e3749a
Reviewed-on: https://chromium-review.googlesource.com/1079721
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
Commit-Queue: Nodir Turakulov <nodir@chromium.org>

[modify] https://crrev.com/6942a0b15aa9cc5c504a5df4ef5140a2947027db/appengine/components/PRESUBMIT.py
[add] https://crrev.com/6942a0b15aa9cc5c504a5df4ef5140a2947027db/appengine/components/components/prpc/discovery/test/Makefile
[add] https://crrev.com/6942a0b15aa9cc5c504a5df4ef5140a2947027db/appengine/components/components/prpc/discovery/test/__init__.py
[add] https://crrev.com/6942a0b15aa9cc5c504a5df4ef5140a2947027db/appengine/components/components/prpc/discovery/test/imported1_test.proto
[add] https://crrev.com/6942a0b15aa9cc5c504a5df4ef5140a2947027db/appengine/components/components/prpc/discovery/test/imported1_test_pb2.py
[add] https://crrev.com/6942a0b15aa9cc5c504a5df4ef5140a2947027db/appengine/components/components/prpc/discovery/test/imported2_test.proto
[add] https://crrev.com/6942a0b15aa9cc5c504a5df4ef5140a2947027db/appengine/components/components/prpc/discovery/test/imported2_test_pb2.py
[add] https://crrev.com/6942a0b15aa9cc5c504a5df4ef5140a2947027db/appengine/components/components/prpc/discovery/test/test.proto
[add] https://crrev.com/6942a0b15aa9cc5c504a5df4ef5140a2947027db/appengine/components/components/prpc/discovery/test/test_pb2.py
[add] https://crrev.com/6942a0b15aa9cc5c504a5df4ef5140a2947027db/appengine/components/components/prpc/discovery/test/test_prpc_pb2.py
[add] https://crrev.com/6942a0b15aa9cc5c504a5df4ef5140a2947027db/appengine/components/components/prpc/discovery/test_support

Comment 3 by s...@google.com, May 31 2018

cr-buildbucket-dev is currently broken, I think because of this.

Comment 4 by no...@chromium.org, May 31 2018

It is broken because gae deployment got stuck. I’ll fix it when I’m at computer again 

Comment 5 by s...@google.com, May 31 2018

New deployments themselves are broken. I tried deploying cr-buildbucket-dev with some endpoints-related changes and I got:

Traceback (most recent call last):
  File "appengine/runtime/wsgi.py", line 240, in Handle
    handler = _config_handle.add_wsgi_middleware(self._LoadHandler())
  File "appengine/runtime/wsgi.py", line 299, in _LoadHandler
    handler, path, err = LoadObject(self._handler)
  File "appengine/runtime/wsgi.py", line 85, in LoadObject
    obj = __import__(path[0])
  File "apps.py", line 18, in <module>
    html, endpoints, backend = main.initialize()
  File "main.py", line 47, in initialize
    return create_html_app(), create_endpoints_app(), create_backend_app()
  File "main.py", line 22, in create_html_app
    handlers.get_frontend_routes(), debug=utils.is_local_dev_server())
  File "handlers.py", line 349, in get_frontend_routes
    prpc_server.add_service(access.AccessServicer())
  File "components/prpc/server.py", line 138, in add_service
    self._discovery_service.add_service(servicer.DESCRIPTION)
  File "components/prpc/discovery/service.py", line 53, in add_service
    ensure_file(service_description['file'])
KeyError: 'file'
Note: this has brought down config validation for projects that have cr-buildbucket-dev.cfg file: on config changes, luci-config tries to access cr-buildbucket-dev's validation endpoint and fails. This results in:

CRITICAL: cr-buildbucket-dev.cfg: Error during external validation: Net error: Failed to call https://cr-buildbucket-dev.appspot.com/_ah/api/config/v1/validate after 4 attempts url: https://cr-buildbucket-dev.appspot.com/_ah/api/config/v1/validate config_set: projects/fuchsia path: cr-buildbucket-dev.cfg response: None

That blocks the config change.

I'll try to revert cr-buildbucket-dev to a working version.
Switched it to 15697-789ade6 deployed on May 25.
API Explorer doesn't work in this version :-/ So I assume config validation will be broken too.

Switched to even older 15637-249eaa8 from May 22. API Explorer works...
Project Member

Comment 9 by bugdroid1@chromium.org, May 31 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/65d1362c39f8ff7caa09e94da6c2eb105aec505b

commit 65d1362c39f8ff7caa09e94da6c2eb105aec505b
Author: Nodir Turakulov <nodir@chromium.org>
Date: Thu May 31 04:31:01 2018

Revert "[prpc] Add missing discovery/test"

This reverts commit 6942a0b15aa9cc5c504a5df4ef5140a2947027db.

Reason for revert: https://chromium-review.googlesource.com/c/infra/luci/luci-py/+/1074197 will be reverted

Original change's description:
> [prpc] Add missing discovery/test
> 
> https://chromium-review.googlesource.com/c/infra/luci/luci-py/+/1074197
> got landed with broken tests (some files were missing).
> PRESUBMIT didn't fail.
> 
> Update presubmit to run prpc tests. Add the missing files.
> 
> Bug:  846935 
> Change-Id: I4c52fbb6a4e8469cec1c11ede2585fa323e3749a
> Reviewed-on: https://chromium-review.googlesource.com/1079721
> Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
> Commit-Queue: Nodir Turakulov <nodir@chromium.org>

TBR=vadimsh@chromium.org,nodir@chromium.org

Change-Id: I1ab62a219336014d7c8b76a7a169515c335a0ec4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  846935 
Reviewed-on: https://chromium-review.googlesource.com/1079733
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: Nodir Turakulov <nodir@chromium.org>

[modify] https://crrev.com/65d1362c39f8ff7caa09e94da6c2eb105aec505b/appengine/components/PRESUBMIT.py
[delete] https://crrev.com/ee8ee5a8cc91b2361e86308324934e6be7db5dc7/appengine/components/components/prpc/discovery/test/Makefile
[delete] https://crrev.com/ee8ee5a8cc91b2361e86308324934e6be7db5dc7/appengine/components/components/prpc/discovery/test/__init__.py
[delete] https://crrev.com/ee8ee5a8cc91b2361e86308324934e6be7db5dc7/appengine/components/components/prpc/discovery/test/imported1_test.proto
[delete] https://crrev.com/ee8ee5a8cc91b2361e86308324934e6be7db5dc7/appengine/components/components/prpc/discovery/test/imported1_test_pb2.py
[delete] https://crrev.com/ee8ee5a8cc91b2361e86308324934e6be7db5dc7/appengine/components/components/prpc/discovery/test/imported2_test.proto
[delete] https://crrev.com/ee8ee5a8cc91b2361e86308324934e6be7db5dc7/appengine/components/components/prpc/discovery/test/imported2_test_pb2.py
[delete] https://crrev.com/ee8ee5a8cc91b2361e86308324934e6be7db5dc7/appengine/components/components/prpc/discovery/test/test.proto
[delete] https://crrev.com/ee8ee5a8cc91b2361e86308324934e6be7db5dc7/appengine/components/components/prpc/discovery/test/test_pb2.py
[delete] https://crrev.com/ee8ee5a8cc91b2361e86308324934e6be7db5dc7/appengine/components/components/prpc/discovery/test/test_prpc_pb2.py
[delete] https://crrev.com/ee8ee5a8cc91b2361e86308324934e6be7db5dc7/appengine/components/components/prpc/discovery/test_support

Project Member

Comment 10 by bugdroid1@chromium.org, May 31 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/d73cc5090410de0b761969ee49cfee065b48b613

commit d73cc5090410de0b761969ee49cfee065b48b613
Author: Nodir Turakulov <nodir@chromium.org>
Date: Thu May 31 04:32:01 2018

Revert "[prpc] Add discovery service"

This reverts commit bd85d6af3a4ae25aabaa18f172bb39a2662989c2.

Reason for revert: this impl doesn't provide source code info

Original change's description:
> [prpc] Add discovery service
> 
> This will enable RPC Explorer for Buildbucket API V2.
> Also prpc cmdline tool.
> 
> Bug:  846935 
> Change-Id: I5876b72b5bfeb8f207dfb4a80f0ac06f8fb958b9
> Reviewed-on: https://chromium-review.googlesource.com/1074197
> Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
> Commit-Queue: Nodir Turakulov <nodir@chromium.org>

TBR=vadimsh@chromium.org,nodir@chromium.org,mknyszek@google.com

Change-Id: Idf04d28d06d69a0b4327e08e0f7aa536e307413d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  846935 
Reviewed-on: https://chromium-review.googlesource.com/1079734
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: Nodir Turakulov <nodir@chromium.org>

[delete] https://crrev.com/65d1362c39f8ff7caa09e94da6c2eb105aec505b/appengine/components/components/prpc/discovery/README.md
[delete] https://crrev.com/65d1362c39f8ff7caa09e94da6c2eb105aec505b/appengine/components/components/prpc/discovery/__init__.py
[delete] https://crrev.com/65d1362c39f8ff7caa09e94da6c2eb105aec505b/appengine/components/components/prpc/discovery/service.proto
[delete] https://crrev.com/65d1362c39f8ff7caa09e94da6c2eb105aec505b/appengine/components/components/prpc/discovery/service.py
[delete] https://crrev.com/65d1362c39f8ff7caa09e94da6c2eb105aec505b/appengine/components/components/prpc/discovery/service_pb2.py
[delete] https://crrev.com/65d1362c39f8ff7caa09e94da6c2eb105aec505b/appengine/components/components/prpc/discovery/service_prpc_pb2.py
[delete] https://crrev.com/65d1362c39f8ff7caa09e94da6c2eb105aec505b/appengine/components/components/prpc/discovery/service_test.py
[modify] https://crrev.com/d73cc5090410de0b761969ee49cfee065b48b613/appengine/components/components/prpc/server.py
[modify] https://crrev.com/d73cc5090410de0b761969ee49cfee065b48b613/appengine/components/components/prpc/test/test_prpc_pb2.py
[modify] https://crrev.com/d73cc5090410de0b761969ee49cfee065b48b613/appengine/components/tools/protoc-gen-prpc-python

Project Member

Comment 11 by bugdroid1@chromium.org, Jun 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-py.git/+/c06a11eb81b02a9b5da6c3a3c599d3710fcafa8c

commit c06a11eb81b02a9b5da6c3a3c599d3710fcafa8c
Author: Nodir Turakulov <nodir@google.com>
Date: Fri Jun 01 00:00:26 2018

[prpc] Include source code info in _prpc_pb2.py

Change format of _prpc_pb2.py files:
- include FILE_DESCRIPTOR_SET variable that includes description of current
  file and all of its transitive dependencies
- also, include source code info, e.g. comments
- remove descriptors from service description variables.
  Instead only refer to a descriptor in the FILE_DESCRIPTOR_SET
- use base64 and zlib, not hex
- split byte string literal to 80col lines.

This is a pure refactoring, preparation for discovery service.

Bug:  846935 
Change-Id: Ie5e89e170177aeaea25c39845041a13d37264df6
Reviewed-on: https://chromium-review.googlesource.com/1080281
Commit-Queue: Nodir Turakulov <nodir@chromium.org>
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/c06a11eb81b02a9b5da6c3a3c599d3710fcafa8c/appengine/components/components/prpc/server.py
[modify] https://crrev.com/c06a11eb81b02a9b5da6c3a3c599d3710fcafa8c/appengine/components/components/prpc/test/test_prpc_pb2.py
[modify] https://crrev.com/c06a11eb81b02a9b5da6c3a3c599d3710fcafa8c/appengine/components/tools/protoc-gen-prpc-python

Status: Fixed (was: Started)

Sign in to add a comment