Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add python binding #26

Merged
merged 5 commits into from
Sep 24, 2021
Merged

add python binding #26

merged 5 commits into from
Sep 24, 2021

Conversation

robertchoi80
Copy link
Contributor

@robertchoi80 robertchoi80 commented Sep 2, 2021

  • rename proto package to share it among multiple languages (golang and python)
  • generate python binding

본 PR은 openinfradev/decapod-flow#56 과 관련된 PR 입니다.

@zugwan
Copy link
Contributor

zugwan commented Sep 3, 2021

output 디렉토리를 tks를 포함하도록 변경하였는데 github.com/openinfradev/tks-proto/pbgo 처럼 저장소에 tks 가 드러나기 때문에 포함하지 않는 것이 더 좋지 않을까 의견 드립니다. 현 이름 형식 반영해서 아래 정도면 어떨까요?

  • golang: pbgo
  • python: pbpy

zugwan
zugwan previously requested changes Sep 3, 2021
Makefile Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Sep 6, 2021

This PR is stale because it has been open 3 days with no activity. Remove stale label or comment or this will be closed in 3 days.

@github-actions github-actions bot added the Stale There has been no activity on this label Sep 6, 2021
Makefile Show resolved Hide resolved
@github-actions
Copy link

This PR was closed because it has been stalled for 10 days with no activity.

@github-actions github-actions bot closed this Sep 12, 2021
@robertchoi80
Copy link
Contributor Author

robertchoi80 commented Sep 13, 2021

output 디렉토리를 tks를 포함하도록 변경하였는데 github.com/openinfradev/tks-proto/pbgo 처럼 저장소에 tks 가 드러나기 때문에 포함하지 않는 것이 더 좋지 않을까 의견 드립니다. 현 이름 형식 반영해서 아래 정도면 어떨까요?

  • golang: pbgo
  • python: pbpy

우선, package명 자체는 언어 독립적으로 common하게 네이밍되어야 할 듯 합니다. 제가 코멘트 남긴 부분 (pointA) 보시면, 빌드된 rpc 코드 쪽에 package명이 그대로 박히기 때문에, 이름 자체는 'go'나 'py' 같은 부분이 들어가면 안될 듯 하구요.
그렇다고 해서 그냥 'pb'로 하게 되면, go 코드 입장에선 'github.com/openinfradev/tks-proto/pb' 처럼 tks 부분이 인지되지만, python client 입장에선 단지 'pb' 가 되어 좀 모호해질 수 있습니다. (사실 동일 코드 내에서 다른 proto를 사용할 가능성은 희박하므로 크게 상관 없을 거 같긴 합니다만..)

output 디렉토리를 tks_pb와 tks_pb_python 으로 명명한 이유는,

  • golang의 경우 패키지명과 디렉토리명이 같아야 한다는 제약이 있어 go output 디렉토리를 패키지명과 동일하게 하였습니다. (또한 tks-service 들에서 직접 해당 디렉토리 경로를 참조해서 사용하므로, 어찌 보면 TKS의 대표 RPC output 코드라 볼 수 있지 않을까 생각했구요)
  • python output의 경우는 해당 디렉토리를 직접 참조하지 않고, 어차피 clone하여 workflow 쪽에 마운트하거나 직접 image 내에 넣어 사용할 것이므로 네이밍은 크게 문제가 되지 않았습니다.

이렇게 하긴 했는데, 아무래도 일관성이 있진 않아서 더 좋은 아이디어 있으면 제안 부탁드립니다.

@robertchoi80 robertchoi80 reopened this Sep 13, 2021
and rename proto package to share it among multiple languages
@github-actions github-actions bot removed the Stale There has been no activity on this label Sep 13, 2021
@robertchoi80 robertchoi80 changed the title [WIP] add python binding add python binding Sep 13, 2021
@ktkfree ktkfree requested review from ktkfree and zugwan September 14, 2021 02:19
@Jaesang
Copy link

Jaesang commented Sep 15, 2021

python에서 사용할 때 어떻게 임포트되나요?

from tks-proto import pb 

이런식이면 모호하진 않을것 같은데, 실제로 어떻게 임포트되는지 모르겠네요

그렇다고 해서 그냥 'pb'로 하게 되면, go 코드 입장에선 'github.com/openinfradev/tks-proto/pb' 처럼 tks 부분이 인지되지만, python client 입장에선 단지 'pb' 가 되어 좀 모호해질 수 있습니다. (사실 동일 코드 내에서 다른 proto를 사용할 가능성은 희박하므로 크게 상관 없을 거 같긴 합니다만..)

@robertchoi80
Copy link
Contributor Author

robertchoi80 commented Sep 16, 2021

python에서 사용할 때 어떻게 임포트되나요?

from tks-proto import pb 

이런식이면 모호하진 않을것 같은데, 실제로 어떻게 임포트되는지 모르겠네요

그렇다고 해서 그냥 'pb'로 하게 되면, go 코드 입장에선 'github.com/openinfradev/tks-proto/pb' 처럼 tks 부분이 인지되지만, python client 입장에선 단지 'pb' 가 되어 좀 모호해질 수 있습니다. (사실 동일 코드 내에서 다른 proto를 사용할 가능성은 희박하므로 크게 상관 없을 거 같긴 합니다만..)

tks-proto가 전부 필요하진 않고, client 입장에선 생성된 python gRPC 코드만 있으면 됩니다. (tks_pb_python 디렉토리) 일단 자동 생성된 python 코드를 보면 package='tks_pb' 같은 부분이 들어가있어 궁극적으로는 tks_pb 라고 python package를 만들고 아래와 같이 사용하는 게 좋을 것 같은데,,

from tks_pb import info_pb2
from tks_pb import common_pb2

현재는 아직 package 생성까지는 안했고, 그냥 각각의 모듈들을 개별 import하는 식으로 쓰고 있습니다. 해당 모듈들은 PYTHONPATH 내에 넣어두고요.

    import info_pb2
    import info_pb2_grpc
    ...       

python packge화하려면 init.py 파일을 만들고 몇가지 절차가 필요한 듯 한데, 이 부분은 TODO 로 생각하고 있습니다.

@github-actions
Copy link

This PR is stale because it has been open 3 days with no activity. Remove stale label or comment or this will be closed in 3 days.

@github-actions github-actions bot added the Stale There has been no activity on this label Sep 19, 2021
@github-actions
Copy link

This PR was closed because it has been stalled for 10 days with no activity.

@github-actions github-actions bot closed this Sep 22, 2021
@robertchoi80 robertchoi80 reopened this Sep 24, 2021
@bluejayA bluejayA removed the Stale There has been no activity on this label Sep 24, 2021
@ktkfree ktkfree merged commit 178698d into main Sep 24, 2021
@ktkfree ktkfree deleted the add_python_binding branch September 24, 2021 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants