Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Improve proto generation and cleanup old protos #281

Merged
merged 4 commits into from
Jun 2, 2021
Merged

Conversation

albscui
Copy link

@albscui albscui commented May 27, 2021

While attempting to move proto generation to pachyderm/pachyderm, we've learned a few things and improved the scripts for this process. However, we decided that proto generation and packaging is a bigger project that involves the organization as a whole, so we won't be migrating proto generation from python-pachyderm to pachyderm/pachyderm atm. Related issue here pachyderm/pachyderm#6116.

However, we can still benefit from this exercise and apply some of the improvements:

  • Simplified and more valid proto/run for generating protos
  • Parameterize the proto generator's output dir
    • this will allow us to generate different protos based on different versions of pachyderm/pachyderm
  • Slimmer Docker image using python:3.6-slim

We are also adding a new python_pachyderm.proto.v2 namespace to store the protos generated from pachyderm v2.x

We removed existing generated protos from python_pachyderm/proto.

Next steps

  • We can generate a set of protos based off of pachyderm/pachyderm v1.x
  • Update python_pachyderm/__init__.py to defer importing a specific proto version, so that python_pachyderm.proto.v1 and python_pachyderm.proto.v2 can co-exist in the same repo

Relates to #282

@albscui albscui requested a review from msteffen May 27, 2021 15:52
@albscui albscui force-pushed the albert/proto-v2 branch from c2fb248 to 3bf7dbb Compare May 31, 2021 17:02
Copy link
Contributor

@msteffen msteffen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@albscui albscui merged commit b09f355 into master Jun 2, 2021
@albscui albscui deleted the albert/proto-v2 branch June 2, 2021 20:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants