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

[SDK] Fix the tricky import changes in Python SDK #2416

Open
Electronic-Waste opened this issue Sep 1, 2024 · 4 comments
Open

[SDK] Fix the tricky import changes in Python SDK #2416

Electronic-Waste opened this issue Sep 1, 2024 · 4 comments

Comments

@Electronic-Waste
Copy link
Member

What you would like to be added?

This issue is created to track the tricky import changes in Python SDK.

Currently, we have no choice but to do the tricky import changes because: #2398 (comment) .

katib/Makefile

Lines 169 to 179 in a524f33

# TODO(Electronic-Waste): Remove the import rewrite when protobuf supports `python_package` option.
# REF: https://github.com/protocolbuffers/protobuf/issues/7061
pytest: prepare-pytest prepare-pytest-testdata
pytest ./test/unit/v1beta1/suggestion --ignore=./test/unit/v1beta1/suggestion/test_skopt_service.py
pytest ./test/unit/v1beta1/earlystopping
pytest ./test/unit/v1beta1/metricscollector
cp ./pkg/apis/manager/v1beta1/python/api_pb2.py ./sdk/python/v1beta1/kubeflow/katib/katib_api_pb2.py
cp ./pkg/apis/manager/v1beta1/python/api_pb2_grpc.py ./sdk/python/v1beta1/kubeflow/katib/katib_api_pb2_grpc.py
sed -i "s/api_pb2/kubeflow\.katib\.katib_api_pb2/g" ./sdk/python/v1beta1/kubeflow/katib/katib_api_pb2_grpc.py
pytest ./sdk/python/v1beta1/kubeflow/katib
rm ./sdk/python/v1beta1/kubeflow/katib/katib_api_pb2.py ./sdk/python/v1beta1/kubeflow/katib/katib_api_pb2_grpc.py

# TODO(Electronic-Waste): Remove the import rewrite when protobuf supports `python_package` option.
# REF: https://github.com/protocolbuffers/protobuf/issues/7061
if os.path.exists(katib_grpc_svc_file):
shutil.copy(
katib_grpc_svc_file,
"kubeflow/katib/katib_api_pb2_grpc.py",
)
with open("kubeflow/katib/katib_api_pb2_grpc.py", "r+") as file:
content = file.read()
new_content = content.replace("api_pb2", "kubeflow.katib.katib_api_pb2")
file.seek(0)
file.write(new_content)
file.truncate()

Why is this needed?

As I described in #2398 (comment) , we need to discard the tricky import changes when protocolbuffers/protobuf#7061 supports options like python_package to mutate the importing package name.

Love this feature?

Give it a 👍 We prioritize the features with most 👍

@andreyvelich
Copy link
Member

/remove-label lifecycle/needs-triage
/area sdk

@tenzen-y
Copy link
Member

tenzen-y commented Sep 2, 2024

@kubeflow/wg-automl-leads Protocol Buffers code generator is extensible. So, TBH, I'm wondering if we can implement custom Python protoc plugin for Katib since it seems that protocolbuffers/protobuf#7061 was opened 4 years ago, and then the issue is not still resolved...

Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@Electronic-Waste
Copy link
Member Author

/remove-lifecycle stale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants