Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Migrate to go module and remove old Hive integration tests #36

Merged
merged 8 commits into from
Jan 27, 2020

Conversation

honnix
Copy link
Member

@honnix honnix commented Nov 28, 2019

Some tests are being removed because they need to be updated and that breaks go mod. Can add back in the future. For some reason go mod doesn't know how to respect build tags.

@honnix
Copy link
Member Author

honnix commented Nov 28, 2019

@kumare3 PTAL

kumare3
kumare3 previously approved these changes Nov 30, 2019
Copy link
Contributor

@kumare3 kumare3 left a comment

Choose a reason for hiding this comment

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

Awesome!

@honnix
Copy link
Member Author

honnix commented Jan 21, 2020

I had to delete some test files under tests because they are not compilable.

wild-endeavor added a commit to flyteorg/boilerplate that referenced this pull request Jan 23, 2020
This updates the boilerplate to use go modules instead of dep and follows the changes that have already been made:

* flyteorg/datacatalog#21
* flyteorg/flytepropeller#38
* flyteorg/flyteadmin#35
* flyteorg/flyteplugins#36
* flyteorg/flytestdlib#50
* flyteorg/flyteidl#27 (depends on flyteorg/flytestdlib#50 to be merged and released)
@honnix
Copy link
Member Author

honnix commented Jan 24, 2020

@wild-endeavor PTAL. I had to delete those non-compilable code otherwise go module is sad.

@wild-endeavor
Copy link
Contributor

I think it's fine - those tests were written a while ago and then not kept up to date since we still don't have a cohesive integration testing story. Let me change the title of the PR though to make them easier to find in case I ever want to resurrect them.

@wild-endeavor wild-endeavor changed the title migrate to go module Migrate to go module and remove old Hive integration tests Jan 24, 2020
EngHabu
EngHabu previously approved these changes Jan 27, 2020
go.mod Outdated Show resolved Hide resolved
@@ -1,201 +0,0 @@
// +build manualintegration
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to delete this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think so, but they're not working anymore. I should try to fix them at some point.

k8s.io/client-go v11.0.0+incompatible
k8s.io/klog v1.0.0 // indirect
k8s.io/utils v0.0.0-20200122174043-1e243dd1a584 // indirect
sigs.k8s.io/controller-runtime v0.4.0
Copy link
Contributor

Choose a reason for hiding this comment

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

We used to depend on a SHA, is this version inclusive?

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/kubernetes-sigs/controller-runtime/releases/tag/v0.4.0 includes sha 40070e2a1958c3d974ba95da883a2bd088137789

@honnix
Copy link
Member Author

honnix commented Jan 27, 2020

I ran go mod tidy on this PR and go.sum got modified. Can you try that as well @wild-endeavor ?

@wild-endeavor
Copy link
Contributor

ran go mod tidy as well.

go.mod Show resolved Hide resolved
@wild-endeavor wild-endeavor merged commit 990ce7e into flyteorg:master Jan 27, 2020
@honnix honnix deleted the go-module-migration branch January 27, 2020 21:06
eapolinario pushed a commit to eapolinario/flyte that referenced this pull request Aug 21, 2023
This updates the boilerplate to use go modules instead of dep and follows the changes that have already been made:

* flyteorg/datacatalog#21
* flyteorg/flytepropeller#38
* flyteorg/flyteadmin#35
* flyteorg/flyteplugins#36
* flyteorg/flytestdlib#50
* flyteorg/flyteidl#27 (depends on flyteorg/flytestdlib#50 to be merged and released)
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
migrate to go mod and temporarily remove some broken tests
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.

4 participants