-
Notifications
You must be signed in to change notification settings - Fork 659
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
Update helm/docs per changes in supported task discovery #5694
Update helm/docs per changes in supported task discovery #5694
Conversation
eac6e67
to
db28350
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5694 +/- ##
==========================================
- Coverage 36.17% 36.17% -0.01%
==========================================
Files 1303 1303
Lines 109663 109663
==========================================
- Hits 39667 39666 -1
- Misses 65851 65852 +1
Partials 4145 4145
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Pinging @davidmirror-ops since he has more expertise in the helm charts & deployment docs than me or Peeter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here's some todo:
- solve the merged conflict
- before we merge it, we have to build an image of flyte and test it.
go to this folder and run
make build
https://github.com/flyteorg/flyte/tree/master/docker/sandbox-bundled
if possible, please run a sensor task and provide screenshot that new image works!
https://docs.flyte.org/en/latest/flytesnacks/examples/sensor/file_sensor_example.html
Signed-off-by: Jason Parraga <[email protected]>
Signed-off-by: Jason Parraga <[email protected]>
bda86fd
to
f1ecfd9
Compare
I have updated the PR description and seems that I was able to get the task running, but not sure how to satisfy the sensor so the task completes. But I can verify that propeller identifies the agent-service for the task type. |
6f52211
to
fa18557
Compare
Signed-off-by: Jason Parraga <[email protected]>
fa18557
to
cefad53
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you @Sovietaced
* Update helm/docs per changes in supported task discovery Signed-off-by: Jason Parraga <[email protected]> * Cleanup flyte-binary Signed-off-by: Jason Parraga <[email protected]> * fixes Signed-off-by: Jason Parraga <[email protected]> --------- Signed-off-by: Jason Parraga <[email protected]> Signed-off-by: pmahindrakar-oss <[email protected]>
* Update helm/docs per changes in supported task discovery Signed-off-by: Jason Parraga <[email protected]> * Cleanup flyte-binary Signed-off-by: Jason Parraga <[email protected]> * fixes Signed-off-by: Jason Parraga <[email protected]> --------- Signed-off-by: Jason Parraga <[email protected]> Signed-off-by: pmahindrakar-oss <[email protected]>
* Update helm/docs per changes in supported task discovery Signed-off-by: Jason Parraga <[email protected]> * Cleanup flyte-binary Signed-off-by: Jason Parraga <[email protected]> * fixes Signed-off-by: Jason Parraga <[email protected]> --------- Signed-off-by: Jason Parraga <[email protected]> Signed-off-by: Bugra Gedik <[email protected]>
Why are the changes needed?
These changes aren't strictly needed but they simplify configuration based on the changes introduced in #5460
What changes were proposed in this pull request?
Removes explicit configuration supported task types since they are now automatically discovered.
How was this patch tested?
Built and ran the sandbox bundled. Executed a file sensor workflow.
Seen in the logs..
Check all the applicable boxes
Related PRs
Docs link