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.

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

[wip] artf svc & debugging #4475

Closed
wants to merge 19 commits into from
Closed

[wip] artf svc & debugging #4475

wants to merge 19 commits into from

Conversation

wild-endeavor
Copy link
Contributor

add back all files according to 9d7e55e

Copy link

codecov bot commented Nov 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ce5cb7b) 58.11% compared to head (8e8f1cf) 56.82%.

Additional details and impacted files
@@                 Coverage Diff                 @@
##           artifacts-shell    #4475      +/-   ##
===================================================
- Coverage            58.11%   56.82%   -1.29%     
===================================================
  Files                  626       34     -592     
  Lines                53815     2154   -51661     
===================================================
- Hits                 31273     1224   -30049     
+ Misses               20039      837   -19202     
+ Partials              2503       93    -2410     
Flag Coverage Δ
unittests 56.82% <ø> (-1.29%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

squiishyy and others added 5 commits December 7, 2023 16:26
* add enable_in_tree_autoscaling

Signed-off-by: Yicheng-Lu-llll <[email protected]>

* remove print

Signed-off-by: Yicheng-Lu-llll <[email protected]>

* allow MinReplicas to be zero

Signed-off-by: Yicheng-Lu-llll <[email protected]>

* fix ci test

Signed-off-by: Yicheng-Lu-llll <[email protected]>

* fix ci test

Signed-off-by: Yicheng-Lu-llll <[email protected]>

* add ttl

Signed-off-by: Yicheng-Lu-llll <[email protected]>

* nit

Signed-off-by: Yicheng-Lu-llll <[email protected]>

* change JobDeploymentStatusFailedToGetJobStatus -> PhaseInfoRunning

Signed-off-by: Yicheng-Lu-llll <[email protected]>

* rebasing

Signed-off-by: Yicheng-Lu-llll <[email protected]>

* rebasing

Signed-off-by: Yicheng-Lu-llll <[email protected]>

* enforce minreplica < replica < maxreplica

Signed-off-by: Yicheng-Lu-llll <[email protected]>

* add test for ShutdownAfterJobFinishes and TTLSecondsAfterFinished

Signed-off-by: Yicheng-Lu-llll <[email protected]>

* fix ray test error

Signed-off-by: Yicheng-Lu-llll <[email protected]>

---------

Signed-off-by: Yicheng-Lu-llll <[email protected]>
Co-authored-by: Kevin Su <[email protected]>
* use-personal

* tidy

* update port to 30004

Signed-off-by: Yee Hing Tong <[email protected]>

* Artifacts bundle (#7)

* Remove private repos (#4078)

Signed-off-by: Yee Hing Tong <[email protected]>

* nit from monorepo merge

Signed-off-by: Yee Hing Tong <[email protected]>

* Artifacts bring in idl (#4204)

Signed-off-by: Yee Hing Tong <[email protected]>

* Artifacts bring admin monorepo (#4203)

Main pr to bring in the boilerplate code for artifact service into the main pr.

Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>

* Artf/triggers (#4394)

This PR adds the trigger concept.

Other changes:
* Forgot to make the index on artifactkey unique.
* Fix to create artifact, it wasn't pulling back out the artifact key.
* Updated the Artifact service to handle the passing of artifacts created by the events processor to the trigger handling components. Currently, and this is potentially a design flaw, the events processor has no way of contacting the trigger component directly.

Signed-off-by: Yee Hing Tong <[email protected]>

* make trigger key index unique (#4410)

Signed-off-by: Yee Hing Tong <[email protected]>

* revert data proxy changes (#4411)

Signed-off-by: Yee Hing Tong <[email protected]>

* artf/ fix issue with trigger (#4412)

Signed-off-by: Yee Hing Tong <[email protected]>

* sandbox lite dockerfile (#4413)

Signed-off-by: Yee Hing Tong <[email protected]>

* More pgconn handling fixes (#4420)

Signed-off-by: Yee Hing Tong <[email protected]>

* Artf/switch event (#4428)

Most of the changes here are generated proto changes. Actual code changes are:
IDL
* Remove the supplemental fields in the CloudEventTaskExecution object and move them to CloudEventNodeExecution object.
* Remove some fields that the artifact service ended up not using (parent_node_execution and scheduled_at)

in the cloudevent publisher, change the code filling in of the aforementioned supplemental information to happen for node execution events instead of task execution events.
* Remove the deleted fields.

On the event handling side, move the logic to the handling of the node event instead of the task event.
Signed-off-by: Yee Hing Tong <[email protected]>

* Artf/lints (#4429)

* lint fixes
* rename sandbox_utils without underscore

Signed-off-by: Yee Hing Tong <[email protected]>

* look for other places where pgconn is cast and add handling (#4430)

Add additional handling of pgconn
* Change datacatalog logic to use flytestdlib function instead.
* Change flytestdlib constants to capitalize
* Add handling of the other type to Admin code directly

Signed-off-by: Yee Hing Tong <[email protected]>

* add artifacts branch to buf push (#4448)

Signed-off-by: squiishyy <[email protected]>

* add dummy comment (#4451)

Signed-off-by: squiishyy <[email protected]>

* add dummy comment again (#4453)

Signed-off-by: squiishyy <[email protected]>

* artf/updates to source (#4443)


Signed-off-by: Yee Hing Tong <[email protected]>

* add a feature gate (#4472)


Signed-off-by: Yee Hing Tong <[email protected]>

* fix bug in execution_manager with lpExpectedInputs, remove the standalone artifact client and move into idl clientset, update local config file, remove a deprecated function (#4473)

Signed-off-by: Yee Hing Tong <[email protected]>

* initial deletion of artifact code

Signed-off-by: Yee Hing Tong <[email protected]>

* running make manifests in docker/sandbox-bundled to update generated files

Signed-off-by: Yee Hing Tong <[email protected]>

* now compiles (#4481)

Signed-off-by: Yee Hing Tong <[email protected]>

* add a debug line

Signed-off-by: Yee Hing Tong <[email protected]>

* more debugging

Signed-off-by: Yee Hing Tong <[email protected]>

* clean up go mod and switch the order of the input extraction to check for uri first

Signed-off-by: Yee Hing Tong <[email protected]>

* change artifacts http mappings in protos

Signed-off-by: squiishyy <[email protected]>

* testing removal of go mod pin

Signed-off-by: Yee Hing Tong <[email protected]>

* update go mod some more

Signed-off-by: Yee Hing Tong <[email protected]>

* copilot go mod

Signed-off-by: Yee Hing Tong <[email protected]>

* change artifacts http mappings again (#4562)

Signed-off-by: squiishyy <[email protected]>

* panic and revert gorm log level change

Signed-off-by: Yee Hing Tong <[email protected]>

* revert local to 5

Signed-off-by: Yee Hing Tong <[email protected]>

* make helm

Signed-off-by: Yee Hing Tong <[email protected]>

* Reclaim space prior to running functional tests in single-binary

Signed-off-by: Eduardo Apolinario <[email protected]>

* Pin flytectl version to 0.8.0

Signed-off-by: Eduardo Apolinario <[email protected]>

* Revert "Pin flytectl version to 0.8.0"

This reverts commit cfa4745.

Signed-off-by: Eduardo Apolinario <[email protected]>

* Use flytagent v1.10.2

Signed-off-by: Eduardo Apolinario <[email protected]>

* Debug single-binary

Signed-off-by: Eduardo Apolinario <[email protected]>

* Use an infinite loop instead of watch (term env var is not set)

Signed-off-by: Eduardo Apolinario <[email protected]>

* Get logs from single-binary pod

Signed-off-by: Eduardo Apolinario <[email protected]>

* Debug single-binary pod: use a different way of choosing the pod

Signed-off-by: Eduardo Apolinario <[email protected]>

* Fix flytectl version

Signed-off-by: Eduardo Apolinario <[email protected]>

* Use port 5432 to connect to postgres

Signed-off-by: Eduardo Apolinario <[email protected]>

* Revert "Use port 5432 to connect to postgres"

This reverts commit 8ded67b.

Signed-off-by: Eduardo Apolinario <[email protected]>

* Revert "Fix flytectl version"

This reverts commit 3968d40.

Signed-off-by: Eduardo Apolinario <[email protected]>

* fix database error

Signed-off-by: Yee Hing Tong <[email protected]>

* revert .github/workflows/single-binary.yml by pulling from master

Signed-off-by: Yee Hing Tong <[email protected]>

* Review feedback

Signed-off-by: Eduardo Apolinario <[email protected]>

---------

Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: squiishyy <[email protected]>
Co-authored-by: Joe Eschen <[email protected]>
Co-authored-by: squiishyy <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
An error occurred while trying to automatically change base from artifacts-shell to master December 27, 2023 00:48
eapolinario and others added 13 commits December 26, 2023 17:55
* Stop producing the protoc-gen-validate code in go and python

Signed-off-by: Eduardo Apolinario <[email protected]>

* Remove python files

Signed-off-by: Eduardo Apolinario <[email protected]>

* Remove pb.validate.go files

Signed-off-by: Eduardo Apolinario <[email protected]>

* Add datacatalog files

Signed-off-by: Eduardo Apolinario <[email protected]>

* Remove uses of validate in flyteadmin

Signed-off-by: Eduardo Apolinario <[email protected]>

* Remove validate from artifacts and cloudevents

Signed-off-by: Eduardo Apolinario <[email protected]>

---------

Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
Clean up.

* Remove the inputs/outputs logic from the cloud publisher on the admin side and move it to the event handler on the artifact side
* Remove the initialization sql bit and made hstore extension creation just a migration
* Refactor the handler to be outside of the receiver in artifact events
* Add adminclient into the servicecallhandler object
* Set gizmo sqs config to false for consuming base64 and update the sqs processor to be roughly functional
* Fixing lint issues
* Add artifact configuration to the sandbox values file
* Infra related updates
  * Remove the old python artifact service helm templates
  * update local sandbox configuration to support routing back out to local development endpoint
 
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
make goimports
Update minio address so artifacts can find s3 in sandbox.
make helm

Signed-off-by: Yee Hing Tong <[email protected]>
* 1st stab at updating README

Signed-off-by: davidmirror-ops <[email protected]>

* Expand README

Signed-off-by: davidmirror-ops <[email protected]>

* Remove code snippets

Signed-off-by: davidmirror-ops <[email protected]>

* update readme

Signed-off-by: davidmirror-ops <[email protected]>

* Add note to BUILD section

Signed-off-by: davidmirror-ops <[email protected]>

* Apply 1st round of reviews v2

Signed-off-by: davidmirror-ops <[email protected]>

* Remove question marks

Signed-off-by: davidmirror-ops <[email protected]>

* Unify build and scale sections

Signed-off-by: davidmirror-ops <[email protected]>

* reposition gif

Signed-off-by: davidmirror-ops <[email protected]>

---------

Signed-off-by: davidmirror-ops <[email protected]>
Signed-off-by: David Espejo <[email protected]>
Co-authored-by: Niels Bantilan <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
* handle WaitingForResources phase from backoff controller

Signed-off-by: Daniel Rammer <[email protected]>

* added unit test

Signed-off-by: Daniel Rammer <[email protected]>

---------

Signed-off-by: Daniel Rammer <[email protected]>
* Avoid to use the http.DefaultClient

Signed-off-by: Andres Gomez Ferrer <[email protected]>

* Add httpClient to ClientOptions

Signed-off-by: Andres Gomez Ferrer <[email protected]>

---------

Signed-off-by: Andres Gomez Ferrer <[email protected]>
* updating externalResourceID

Signed-off-by: Daniel Rammer <[email protected]>

* fix unit tests

Signed-off-by: Daniel Rammer <[email protected]>

* generate event recorder mocks

Signed-off-by: Daniel Rammer <[email protected]>

* correctly setting task id in events

Signed-off-by: Daniel Rammer <[email protected]>

---------

Signed-off-by: Daniel Rammer <[email protected]>
…label (#4637)

* Add option in K8s plugin config to inject user identity into pod labels

Signed-off-by: Fabio Graetz <[email protected]>

* Inject user identity into TaskExecutionMetadata labels

Signed-off-by: Fabio Graetz <[email protected]>

* Add unit tests

Signed-off-by: Fabio Graetz <[email protected]>

* Remove duplicate labels injection

Signed-off-by: Fabio Graetz <[email protected]>

* Lint

Signed-off-by: Fabio Graetz <[email protected]>

* Revert "Add option in K8s plugin config to inject user identity into pod labels"

This reverts commit c42a4a0.

Signed-off-by: Fabio Graetz <[email protected]>

* Always inject user identity as pod label if known

Signed-off-by: Fabio Graetz <[email protected]>

* Use hyphen instead of underscore in pod label

Signed-off-by: Fabio Graetz <[email protected]>

* Update flytepropeller/pkg/controller/nodes/task/k8s/task_exec_context.go

Signed-off-by: Fabio M. Graetz, Ph.D. <[email protected]>
Signed-off-by: Fabio Graetz <[email protected]>

* Fix tests

Signed-off-by: Fabio Graetz <[email protected]>

* Remove duplicate unit test logic

Signed-off-by: Fabio Graetz <[email protected]>

---------

Signed-off-by: Fabio Graetz <[email protected]>
Signed-off-by: Fabio M. Graetz, Ph.D. <[email protected]>
Co-authored-by: Dan Rammer <[email protected]>
This is a follow up PR to the [first shell pr](#4474), which just brought in the non-artifact-service artifact changes.  This PR continues with additional smaller changes.

* Remove the `/data` prefix in the grpc gateway paths.
Signed-off-by: Yee Hing Tong <[email protected]>
@wild-endeavor wild-endeavor deleted the branch artifacts-shell January 10, 2024 16:26
@wild-endeavor wild-endeavor deleted the artifacts branch January 22, 2024 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants