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

Add build-tool cli with a crd validation command#3

Open
bnsblue wants to merge 9 commits into
masterfrom
wf-crd-validation
Open

Add build-tool cli with a crd validation command#3
bnsblue wants to merge 9 commits into
masterfrom
wf-crd-validation

Conversation

@bnsblue
Copy link
Copy Markdown
Contributor

@bnsblue bnsblue commented Sep 9, 2019

This PR adds support of crd validation as a command in a new CLI in flytepropeller

@bnsblue
Copy link
Copy Markdown
Contributor Author

bnsblue commented Sep 9, 2019

PTAL @EngHabu @kumare3

Comment thread Gopkg.toml Outdated

[[override]]
name = "k8s.io/kube-openapi"
revision = "743ec37842bffe49dd4221d9026f30fb1d5adbc4"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do they not have versions?

Comment thread Gopkg.toml Outdated

[[constraint]]
name = "github.com/kubeflow/crd-validation"
source = "github.com/bnsblue/crd-validation.git"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's try to get this forked under lyft... if that proved to be too hard, then this is ok we can switch it later.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure. I am waiting on the fork request to be approved...

Comment thread cmd/build-tool/cmd/crd/flyteworkflow.go Outdated
Comment thread cmd/build-tool/cmd/crd_validation.go
return err
}

compilerErrors.SetIncludeSource()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is this for?

err := c.initConfig()
var generator *crd.FlyteWorkflowGenerator
if err != nil {
log.Println("Output will be written to Stdout")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Any reason you don't just fail here instead of trying to "recover" ?

Comment thread cmd/build-tool/main.go Outdated
Comment thread cmd/build-tool/main.go Outdated
fmt.Println(err)
os.Exit(1)
}
} No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
}
}

Comment thread cmd/kubectl-flyte/cmd/root.go Outdated
bnsblue and others added 3 commits September 25, 2019 15:30
Add a new line at the end of the file

Co-Authored-By: Haytham AbuelFutuh <habuelfutuh@lyft.com>
Co-Authored-By: Haytham AbuelFutuh <habuelfutuh@lyft.com>
Co-Authored-By: Haytham AbuelFutuh <habuelfutuh@lyft.com>
@kumare3
Copy link
Copy Markdown
Contributor

kumare3 commented Mar 7, 2020

ping @bnsblue, do we want to move ahead with this?

@bnsblue
Copy link
Copy Markdown
Contributor Author

bnsblue commented Mar 8, 2020

@kumare3 Yes we do. I'll take some time to merge this during the next week

kumare3 pushed a commit to nuclyde-io/flytepropeller that referenced this pull request Feb 4, 2021
* add namepace configuration

* add namespace files

* remove unused var

* update propeller executor config

* use propeller executor configuration

* to lowercase

* fix conflicts

* update namespace test

* remove namespaceMapping variable

* fix compile issues

* add project-domain option and remove incorrect log messages

* upd mock configuration provider

* update unit tests

* Merge logs on task execution event updates (flyteorg#18)

* use fallthrough

* Correct Lint Errors and Add Documentation on Pre-Commit (flyteorg#19)

* README update

* Fix lint errors

* add namepace configuration

* add namespace files

* remove unused var

* update propeller executor config

* use propeller executor configuration

* to lowercase

* fix conflicts

* update namespace test

* remove namespaceMapping variable

* fix compile issues

* add project-domain option and remove incorrect log messages

* upd mock configuration provider

* update unit tests

* use fallthrough

* fix conflicts

* fix more conflicts

* lint

* remove duplicate package

* fix lint errors
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.

3 participants