fix: Change location of the proto-generated python code #2422
Conversation
…east dir Signed-off-by: Achal Shah <achals@gmail.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: achals The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: Achal Shah <achals@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #2422 +/- ##
=======================================
Coverage 85.42% 85.43%
=======================================
Files 122 122
Lines 10731 10735 +4
=======================================
+ Hits 9167 9171 +4
Misses 1564 1564
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Signed-off-by: Achal Shah <achals@gmail.com>
Signed-off-by: Achal Shah <achals@gmail.com>
achals
requested review from
felixwang9817 and
woop
and removed request for
kevjumba
| Type[Int64List], | ||
| Type[StringList], | ||
| ] | ||
| _value_type_proto_value_type_mapping = { |
There was a problem hiding this comment.
Is there a reason for these changes?
| line-length = 88 | ||
| target-version = ['py37'] | ||
| include = '\.pyi?$' | ||
| exclude = ''' |
There was a problem hiding this comment.
What happened to all these exclusions?
| ) | ||
| from feast.protos.feast.core.FeatureView_pb2 import FeatureView as FeatureViewProto | ||
| from feast.protos.feast.core.OnDemandFeatureView_pb2 import ( | ||
| from feast.proto_core.DataSource_pb2 import DataSource as DataSourceProto |
There was a problem hiding this comment.
I am a little bit unclear on what problem you are trying to solve with this change? Didn't we have absolute imports before and after?
There was a problem hiding this comment.
While we did, there was a bug that I found later and fixed in #2437.
With this PR - I just think that the subdir structure is a bit stuttery - feast.protos.feast.*. It's a taste thing, not a major functionality thing.
|
@achals I think there is some merge conflicts that need to be resolved and also willem left some comments! |
|
Doesn't seem like this is causing a problem - and the value in minimal, so I'm going to close this PR for now. |
6 participants
The subdirectory only complicates structures because the proto generated imports are not correct (sine python 3 requires absolute imports, but the proto generated imports start with things like
feast.coreinstead offeast.proto.feast.core.Signed-off-by: Achal Shah achals@gmail.com
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #