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.

Sign up for GitHub

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

Jump to bottom

fix: Prefer installing gopy from feast's fork as opposed to upstream #2839

Merged
merged 7 commits into from
Jun 23, 2022
Merged

fix: Prefer installing gopy from feast's fork as opposed to upstream #2839

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3f56535
fix: Prefer installing gopy from feast's fork as opposed to upstream
achals Jun 22, 2022
c37c576
use go mod tidy
achals Jun 22, 2022
a5445aa
pin and pray
achals Jun 22, 2022
929891d
fix import path
achals Jun 22, 2022
13aa9ba
nix go install
achals Jun 22, 2022
5b107d6
pin go get and add back unpinned go install, and fix other tests
achals Jun 22, 2022
e70cf69
cr
achals Jun 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,12 @@ install-go-proto-dependencies:
go install google.golang.org/grpc/cmd/[email protected]

install-go-ci-dependencies:
# ToDo: currently gopy installation doesn't work w/o explicit go get in the next line
# ToDo: there should be a better way to install gopy
go get github.com/go-python/gopy
# TODO: currently gopy installation doesn't work w/o explicit go get in the next line
# TODO: there should be a better way to install gopy
go get github.com/go-python/gopy@v0.4.0
go install golang.org/x/tools/cmd/goimports
# The `go get` command on the previous lines download the lib along with replacing the dep to `feast-dev/gopy`
# but the following command is needed to install it for some reason.
go install github.com/go-python/gopy
python -m pip install pybindgen==0.22.0

Expand Down
8 changes: 3 additions & 5 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ module github.com/feast-dev/feast

go 1.17

replace github.com/go-python/gopy v0.4.0 => github.com/feast-dev/gopy v0.4.1-0.20220429180328-4257ac71a4d0
Copy link
Collaborator

Choose a reason for hiding this comment

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

was the main change to pin it at v0.40.0? was the re-ordering of “replace” needed in go.mod?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Pinning to v0.4.0 in makefile totally makes sense, since 0.4.0 is not latest version anymore. But this move probably changes nothing.

Copy link
Member Author

Choose a reason for hiding this comment

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

@pyalex is right, the line change has no effect.

Copy link
Member Author

Choose a reason for hiding this comment

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

Pinning was the first main change. The other main change was in setup.py to pass in the go env to go commands. Without that, the go subprocess would behave unexpectedly since the go config would be different since the process environment isn't inherited by default.


require (
github.com/apache/arrow/go/v8 v8.0.0-20220408212425-58fe60f59289
github.com/ghodss/yaml v1.0.0
Expand All @@ -26,8 +28,6 @@ require (
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
github.com/goccy/go-json v0.9.6 // indirect
github.com/golang/snappy v0.0.4 // indirect
github.com/gonuts/commander v0.1.0 // indirect
github.com/gonuts/flag v0.1.0 // indirect
github.com/google/flatbuffers v2.0.6+incompatible // indirect
github.com/klauspost/asmfmt v1.3.2 // indirect
github.com/klauspost/compress v1.15.1 // indirect
Expand All @@ -42,12 +42,10 @@ require (
golang.org/x/net v0.0.0-20220407224826-aac1ed45d8e3 // indirect
golang.org/x/sys v0.0.0-20220406163625-3f8b81556e12 // indirect
golang.org/x/text v0.3.7 // indirect
golang.org/x/tools v0.1.10 // indirect
golang.org/x/tools v0.1.11-0.20220413170336-afc6aad76eb1 // indirect
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
google.golang.org/genproto v0.0.0-20220407144326-9054f6ed7bac // indirect
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect
)

replace github.com/go-python/gopy v0.4.0 => github.com/feast-dev/gopy v0.4.1-0.20220429180328-4257ac71a4d0
6 changes: 2 additions & 4 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,7 @@ github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiu
github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
github.com/golang/snappy v0.0.4 h1:yAGX7huGHXlcLOEtBnF4w7FQwA26wojNCwOYAEhLjQM=
github.com/golang/snappy v0.0.4/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
github.com/gonuts/commander v0.1.0 h1:EcDTiVw9oAVORFjQOEOuHQqcl6OXMyTgELocTq6zJ0I=
github.com/gonuts/commander v0.1.0/go.mod h1:qkb5mSlcWodYgo7vs8ulLnXhfinhZsZcm6+H/z1JjgY=
github.com/gonuts/flag v0.1.0 h1:fqMv/MZ+oNGu0i9gp0/IQ/ZaPIDoAZBOBaJoV7viCWM=
github.com/gonuts/flag v0.1.0/go.mod h1:ZTmTGtrSPejTo/SRNhCqwLTmiAgyBdCkLYhHrAoBdz4=
github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
Expand Down Expand Up @@ -548,8 +546,8 @@ golang.org/x/tools v0.0.0-20201224043029-2b0845dc783e/go.mod h1:emZCQorbCU4vsT4f
golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0=
golang.org/x/tools v0.1.8-0.20211029000441-d6a9af8af023/go.mod h1:nABZi5QlRsZVlzPpHl034qft6wpY4eDcsTt5AaioBiU=
golang.org/x/tools v0.1.9/go.mod h1:nABZi5QlRsZVlzPpHl034qft6wpY4eDcsTt5AaioBiU=
golang.org/x/tools v0.1.10 h1:QjFRCZxdOhBJ/UNgnBZLbNV13DlbnK0quyivTnXJM20=
golang.org/x/tools v0.1.10/go.mod h1:Uh6Zz+xoGYZom868N8YTex3t7RhtHDBrE8Gzo9bV56E=
golang.org/x/tools v0.1.11-0.20220413170336-afc6aad76eb1 h1:Z3vE1sGlC7qiyFJkkDcZms8Y3+yV8+W7HmDSmuf71tM=
golang.org/x/tools v0.1.11-0.20220413170336-afc6aad76eb1/go.mod h1:Uh6Zz+xoGYZom868N8YTex3t7RhtHDBrE8Gzo9bV56E=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
Expand Down
4 changes: 2 additions & 2 deletions sdk/python/tests/integration/online_store/test_e2e_local.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,15 +130,15 @@ def _test_materialize_and_online_retrieval(
cwd=Path(store.repo_path),
)

assert r.returncode == 0
assert r.returncode == 0, f"stdout: {r.stdout}\n stderr: {r.stderr}"
_assert_online_features(store, driver_df, end_date - timedelta(days=7))

# Test `feast materialize-incremental` and online retrieval.
r = runner.run(
["materialize-incremental", end_date.isoformat()], cwd=Path(store.repo_path),
)

assert r.returncode == 0
assert r.returncode == 0, f"stdout: {r.stdout}\n stderr: {r.stderr}"
_assert_online_features(store, driver_df, end_date)


Expand Down
11 changes: 9 additions & 2 deletions sdk/python/tests/utils/cli_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ def local_repo(self, example_repo_py: str, offline_store: str):
path: {data_path / "online_store.db"}
offline_store:
type: {offline_store}
flags:
achals marked this conversation as resolved.
Show resolved Hide resolved
alpha_features: true
on_demand_transforms: true
"""
)
)
Expand All @@ -84,9 +87,13 @@ def local_repo(self, example_repo_py: str, offline_store: str):
repo_example.write_text(example_repo_py)

result = self.run(["apply"], cwd=repo_path)
assert result.returncode == 0
assert (
result.returncode == 0
), f"stdout: {result.stdout}\n stderr: {result.stderr}"

yield FeatureStore(repo_path=str(repo_path), config=None)

result = self.run(["teardown"], cwd=repo_path)
assert result.returncode == 0
assert (
result.returncode == 0
), f"stdout: {result.stdout}\n stderr: {result.stderr}"
6 changes: 4 additions & 2 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,7 @@ def _is_go_ext(self, ext: Extension):
)

def build_extension(self, ext: Extension):
print(f"Building extension {ext}")
if not self._is_go_ext(ext):
# the base class may mutate `self.compiler`
compiler = copy.deepcopy(self.compiler)
Expand All @@ -423,8 +424,8 @@ def build_extension(self, ext: Extension):
)

destination = os.path.dirname(os.path.abspath(self.get_ext_fullpath(ext.name)))
subprocess.check_call(["go", "install", "golang.org/x/tools/cmd/goimports"])
subprocess.check_call(["go", "install", "github.com/go-python/gopy"])
subprocess.check_call(["go", "mod", "tidy"], env={"PATH": bin_path, **go_env})
# subprocess.check_call(["go", "install", "github.com/go-python/gopy@v0.4.0"])
achals marked this conversation as resolved.
Show resolved Hide resolved
achals marked this conversation as resolved.
Show resolved Hide resolved
subprocess.check_call(
[
"gopy",
Expand Down Expand Up @@ -456,6 +457,7 @@ def copy_extensions_to_source(self):
src_dir = os.path.join(self.build_lib, src_dir)

# copy whole directory
print(f"Copying from {src_dir} to {dest_dir}")
copy_tree(src_dir, dest_dir)


Expand Down