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

Fix nil pointer when task plugin load returns error #5622

Merged
merged 2 commits into from
Aug 2, 2024

Conversation

Sovietaced
Copy link
Contributor

I accidentally misconfigured one of my plugins and the result was a nil pointer because pluginCore.LoadPlugin errored and returned a nil cp.

This pull request updates the logic such that it checks the error before operating on cp.

@Sovietaced Sovietaced changed the title Fix nil pointer when plugin loading errors Fix nil pointer when task plugin load returns error Aug 2, 2024
Copy link

codecov bot commented Aug 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 36.17%. Comparing base (b18449e) to head (96fd0ff).
Report is 125 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5622   +/-   ##
=======================================
  Coverage   36.16%   36.17%           
=======================================
  Files        1302     1302           
  Lines      109405   109405           
=======================================
+ Hits        39568    39577    +9     
+ Misses      65701    65693    -8     
+ Partials     4136     4135    -1     
Flag Coverage Δ
unittests-datacatalog 51.37% <ø> (ø)
unittests-flyteadmin 55.32% <ø> (+0.03%) ⬆️
unittests-flytecopilot 12.17% <ø> (ø)
unittests-flytectl 62.28% <ø> (ø)
unittests-flyteidl 7.09% <ø> (ø)
unittests-flyteplugins 53.31% <ø> (ø)
unittests-flytepropeller 41.74% <100.00%> (+<0.01%) ⬆️
unittests-flytestdlib 55.27% <ø> (+0.02%) ⬆️

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.

Signed-off-by: Jason Parraga <[email protected]>
Copy link
Member

@Future-Outlier Future-Outlier left a comment

Choose a reason for hiding this comment

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

LGTM, thank you so much

@Sovietaced Sovietaced enabled auto-merge (squash) August 2, 2024 06:17
@Sovietaced Sovietaced merged commit 89fd084 into flyteorg:master Aug 2, 2024
50 checks passed
bgedik pushed a commit to bgedik/flyte that referenced this pull request Aug 15, 2024
vlibov pushed a commit to vlibov/flyte that referenced this pull request Aug 16, 2024
eapolinario added a commit that referenced this pull request Aug 20, 2024
…ame (#5616)

* Add environment variable for pod name

Signed-off-by: Bugra Gedik <[email protected]>

* [flyteadmin] Refactor panic recovery into middleware (#5546)

* Refactor panic handling to middleware

Signed-off-by: Jason Parraga <[email protected]>

* Remove registration of old panicCounter

Signed-off-by: Jason Parraga <[email protected]>

* Add test coverage

Signed-off-by: Jason Parraga <[email protected]>

---------

Signed-off-by: Jason Parraga <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Snowflake agent Doc (#5620)

* TEST build

Signed-off-by: Future-Outlier <[email protected]>

* remove emphasize-lines

Signed-off-by: Future-Outlier <[email protected]>

* test build

Signed-off-by: Future-Outlier <[email protected]>

* revert

Signed-off-by: Future-Outlier <[email protected]>

---------

Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* [flytepropeller][compiler] Error Handling when Type is not found (#5612)

* FlytePropeller Compiler Avoid Crash when Type not found

Signed-off-by: Future-Outlier <[email protected]>

* Update pingsu's error message advices

Signed-off-by: Future-Outlier <[email protected]>
Co-authored-by: pingsutw  <[email protected]>

* fix lint

Signed-off-by: Future-Outlier <[email protected]>

* Trigger CI

Signed-off-by: Future-Outlier <[email protected]>

* Trigger CI

Signed-off-by: Future-Outlier <[email protected]>

---------

Signed-off-by: Future-Outlier <[email protected]>
Co-authored-by: pingsutw <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Fix nil pointer when task plugin load returns error (#5622)

Signed-off-by: Bugra Gedik <[email protected]>

* Log stack trace when refresh cache sync recovers from panic (#5623)

Signed-off-by: Bugra Gedik <[email protected]>

* use private-key (#5626)

Signed-off-by: Bugra Gedik <[email protected]>

* Explain how Agent Secret Works (#5625)

* first version

Signed-off-by: Future-Outlier <[email protected]>

* update

Signed-off-by: Future-Outlier <[email protected]>

---------

Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Fix typo in execution manager (#5619)

Signed-off-by: ddl-rliu <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Amend Admin to use grpc message size (#5628)

* add send arg

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

* Add acction to remove cache in gh runner

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

* Use correct checked out path

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

* Path in strings

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

* Checkout repo in root

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

* Use the correct path to new action

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

* Do not use gh var in path to clear-action-cache

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

* Remove wrong invocation of clear-action-cache

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

* GITHUB_WORKSPACE is implicit in the checkout action

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

* Refer to local `flyte` directory

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

---------

Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* document the process of setting ttl for a ray cluster (#5636)

Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Add CustomHeaderMatcher to pass additional headers (#5563)

Signed-off-by: Andrew Dye <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Turn flyteidl and flytectl releases into manual gh workflows (#5635)

* Make flyteidl releases go through a manual gh workflow

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

* Make flytectl releases go through a manual gh workflow

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

* Rewrite the documentation for `version` and clarify wording in RELEASE.md

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

---------

Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* docs: fix typo (#5643)

* fix CHANGELOG-v0.2.0.md

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

* fix CHANGELOG-v1.0.2-b1.md

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

* fix CHANGELOG-v1.1.0.md

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

* fix CHANGELOG-v1.3.0.md

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

---------

Signed-off-by: Christina <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Use enable_deck=True in docs (#5645)

Signed-off-by: Bugra Gedik <[email protected]>

* Fix flyteidl release  checkout all tags (#5646)

* Fetch all tags in flyteidl-release.yml

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

* Fix sed expression for npm job

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

---------

Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Install pyarrow in sandbox functional tests (#5647)

Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* docs: add documentation for configuring notifications in GCP (#5545)

* update

Signed-off-by: Desi Hsu <[email protected]>

* dco

Signed-off-by: Desi Hsu <[email protected]>

* dco

Signed-off-by: Desi Hsu <[email protected]>

* typo

Signed-off-by: Desi Hsu <[email protected]>

---------

Signed-off-by: Desi Hsu <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Correct "sucessfile" to "successfile" (#5652)

Signed-off-by: Bugra Gedik <[email protected]>

* Fix ordering for custom template values in cluster resource controller (#5648)

Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Don't error when attempting to trigger schedules for inactive projects (#5649)

* Don't error when attempting to trigger schedules for inactive projects

Signed-off-by: Katrina Rogan <[email protected]>

* regen

Signed-off-by: Katrina Rogan <[email protected]>

---------

Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* fix tests

Signed-off-by: Bugra Gedik <[email protected]>

* change to shorter names

Signed-off-by: Bugra Gedik <[email protected]>

* change to shorter names

Signed-off-by: Bugra Gedik <[email protected]>

* change to shorter names

Signed-off-by: Bugra Gedik <[email protected]>

* change to shorter names

Signed-off-by: Bugra Gedik <[email protected]>

* change to shorter names

Signed-off-by: Bugra Gedik <[email protected]>

* Fix comment symbol

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

* fix one more test

Signed-off-by: Bugra Gedik <[email protected]>

---------

Signed-off-by: Bugra Gedik <[email protected]>
Signed-off-by: Jason Parraga <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: ddl-rliu <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Andrew Dye <[email protected]>
Signed-off-by: Christina <[email protected]>
Signed-off-by: Desi Hsu <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Co-authored-by: Jason Parraga <[email protected]>
Co-authored-by: Future-Outlier <[email protected]>
Co-authored-by: pingsutw <[email protected]>
Co-authored-by: ddl-rliu <[email protected]>
Co-authored-by: Yee Hing Tong <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Andrew Dye <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Christina <[email protected]>
Co-authored-by: Thomas J. Fan <[email protected]>
Co-authored-by: desihsu <[email protected]>
Co-authored-by: ShengYu <[email protected]>
Co-authored-by: Katrina Rogan <[email protected]>
pmahindrakar-oss pushed a commit that referenced this pull request Sep 9, 2024
…ame (#5616)

* Add environment variable for pod name

Signed-off-by: Bugra Gedik <[email protected]>

* [flyteadmin] Refactor panic recovery into middleware (#5546)

* Refactor panic handling to middleware

Signed-off-by: Jason Parraga <[email protected]>

* Remove registration of old panicCounter

Signed-off-by: Jason Parraga <[email protected]>

* Add test coverage

Signed-off-by: Jason Parraga <[email protected]>

---------

Signed-off-by: Jason Parraga <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Snowflake agent Doc (#5620)

* TEST build

Signed-off-by: Future-Outlier <[email protected]>

* remove emphasize-lines

Signed-off-by: Future-Outlier <[email protected]>

* test build

Signed-off-by: Future-Outlier <[email protected]>

* revert

Signed-off-by: Future-Outlier <[email protected]>

---------

Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* [flytepropeller][compiler] Error Handling when Type is not found (#5612)

* FlytePropeller Compiler Avoid Crash when Type not found

Signed-off-by: Future-Outlier <[email protected]>

* Update pingsu's error message advices

Signed-off-by: Future-Outlier <[email protected]>
Co-authored-by: pingsutw  <[email protected]>

* fix lint

Signed-off-by: Future-Outlier <[email protected]>

* Trigger CI

Signed-off-by: Future-Outlier <[email protected]>

* Trigger CI

Signed-off-by: Future-Outlier <[email protected]>

---------

Signed-off-by: Future-Outlier <[email protected]>
Co-authored-by: pingsutw <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Fix nil pointer when task plugin load returns error (#5622)

Signed-off-by: Bugra Gedik <[email protected]>

* Log stack trace when refresh cache sync recovers from panic (#5623)

Signed-off-by: Bugra Gedik <[email protected]>

* use private-key (#5626)

Signed-off-by: Bugra Gedik <[email protected]>

* Explain how Agent Secret Works (#5625)

* first version

Signed-off-by: Future-Outlier <[email protected]>

* update

Signed-off-by: Future-Outlier <[email protected]>

---------

Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Fix typo in execution manager (#5619)

Signed-off-by: ddl-rliu <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Amend Admin to use grpc message size (#5628)

* add send arg

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

* Add acction to remove cache in gh runner

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

* Use correct checked out path

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

* Path in strings

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

* Checkout repo in root

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

* Use the correct path to new action

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

* Do not use gh var in path to clear-action-cache

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

* Remove wrong invocation of clear-action-cache

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

* GITHUB_WORKSPACE is implicit in the checkout action

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

* Refer to local `flyte` directory

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

---------

Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* document the process of setting ttl for a ray cluster (#5636)

Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Add CustomHeaderMatcher to pass additional headers (#5563)

Signed-off-by: Andrew Dye <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Turn flyteidl and flytectl releases into manual gh workflows (#5635)

* Make flyteidl releases go through a manual gh workflow

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

* Make flytectl releases go through a manual gh workflow

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

* Rewrite the documentation for `version` and clarify wording in RELEASE.md

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

---------

Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* docs: fix typo (#5643)

* fix CHANGELOG-v0.2.0.md

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

* fix CHANGELOG-v1.0.2-b1.md

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

* fix CHANGELOG-v1.1.0.md

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

* fix CHANGELOG-v1.3.0.md

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

---------

Signed-off-by: Christina <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Use enable_deck=True in docs (#5645)

Signed-off-by: Bugra Gedik <[email protected]>

* Fix flyteidl release  checkout all tags (#5646)

* Fetch all tags in flyteidl-release.yml

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

* Fix sed expression for npm job

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

---------

Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Install pyarrow in sandbox functional tests (#5647)

Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* docs: add documentation for configuring notifications in GCP (#5545)

* update

Signed-off-by: Desi Hsu <[email protected]>

* dco

Signed-off-by: Desi Hsu <[email protected]>

* dco

Signed-off-by: Desi Hsu <[email protected]>

* typo

Signed-off-by: Desi Hsu <[email protected]>

---------

Signed-off-by: Desi Hsu <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Correct "sucessfile" to "successfile" (#5652)

Signed-off-by: Bugra Gedik <[email protected]>

* Fix ordering for custom template values in cluster resource controller (#5648)

Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* Don't error when attempting to trigger schedules for inactive projects (#5649)

* Don't error when attempting to trigger schedules for inactive projects

Signed-off-by: Katrina Rogan <[email protected]>

* regen

Signed-off-by: Katrina Rogan <[email protected]>

---------

Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Bugra Gedik <[email protected]>

* fix tests

Signed-off-by: Bugra Gedik <[email protected]>

* change to shorter names

Signed-off-by: Bugra Gedik <[email protected]>

* change to shorter names

Signed-off-by: Bugra Gedik <[email protected]>

* change to shorter names

Signed-off-by: Bugra Gedik <[email protected]>

* change to shorter names

Signed-off-by: Bugra Gedik <[email protected]>

* change to shorter names

Signed-off-by: Bugra Gedik <[email protected]>

* Fix comment symbol

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

* fix one more test

Signed-off-by: Bugra Gedik <[email protected]>

---------

Signed-off-by: Bugra Gedik <[email protected]>
Signed-off-by: Jason Parraga <[email protected]>
Signed-off-by: Future-Outlier <[email protected]>
Signed-off-by: ddl-rliu <[email protected]>
Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Andrew Dye <[email protected]>
Signed-off-by: Christina <[email protected]>
Signed-off-by: Desi Hsu <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Co-authored-by: Jason Parraga <[email protected]>
Co-authored-by: Future-Outlier <[email protected]>
Co-authored-by: pingsutw <[email protected]>
Co-authored-by: ddl-rliu <[email protected]>
Co-authored-by: Yee Hing Tong <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Andrew Dye <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Christina <[email protected]>
Co-authored-by: Thomas J. Fan <[email protected]>
Co-authored-by: desihsu <[email protected]>
Co-authored-by: ShengYu <[email protected]>
Co-authored-by: Katrina Rogan <[email protected]>
Signed-off-by: pmahindrakar-oss <[email protected]>
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.

2 participants