Skip to content

vdk-core: resolve library error classification on startup#1241

Merged
mrMoZ1 merged 16 commits intomainfrom
person/mzhivkov/library-issues
Nov 11, 2022
Merged

vdk-core: resolve library error classification on startup#1241
mrMoZ1 merged 16 commits intomainfrom
person/mzhivkov/library-issues

Conversation

@mrMoZ1
Copy link
Copy Markdown
Contributor

@mrMoZ1 mrMoZ1 commented Oct 17, 2022

what: Added functionality which handles ImportErrors, logs the error and writes
termination message via the appropriate internal plugin implementation and exits.

Other approaches tried:

  • researched a library that detects import issues (didn't find any)
  • tried moving the load_setuptools_entrypoints method call (which throws the error)
    a bit further in the call chain, however tests failed (see previous pipelines in this pull req)

why: some user data jobs have been failing with ImportErrors (where a
user import clashes with internal imports, however this crashed the
vdk runtime without writing a termination message.

testing: added unit test.

Signed-off-by: mrMoZ1 mzhivkov@vmware.com

@mrMoZ1 mrMoZ1 requested a review from antoniivanov October 20, 2022 13:17
@mrMoZ1 mrMoZ1 marked this pull request as ready for review October 20, 2022 13:18
Comment thread projects/vdk-core/src/vdk/internal/util/utils.py Outdated
Comment thread projects/vdk-core/src/vdk/internal/plugin/plugin.py Outdated
Comment thread projects/vdk-core/src/vdk/internal/plugin/plugin.py
Comment thread projects/vdk-core/tests/functional/run/test_run_load_setuptools_errors.py Outdated
Comment thread projects/vdk-core/tests/functional/run/test_run_load_setuptools_errors.py Outdated
Comment thread projects/vdk-core/src/vdk/internal/util/utils.py Outdated
@ivakoleva ivakoleva requested a review from murphp15 November 7, 2022 08:32
Comment thread projects/vdk-core/src/vdk/internal/cli_entry.py
Comment thread projects/vdk-core/src/vdk/internal/cli_entry.py Outdated
Comment thread projects/vdk-core/src/vdk/internal/cli_entry.py
Copy link
Copy Markdown
Contributor

@ivakoleva ivakoleva left a comment

Choose a reason for hiding this comment

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

I am okay to merge, my main concern about design still is described in a comment about why doesn't plugin_load_success exist into plugin_registry. Lets elaborate before merging

Comment thread projects/vdk-core/src/vdk/internal/cli_entry.py
Comment thread projects/vdk-core/src/vdk/internal/cli_entry.py Outdated
Comment thread projects/vdk-core/src/vdk/internal/cli_entry.py Outdated
Comment thread projects/vdk-core/tests/functional/run/test_run_load_setuptools_errors.py Outdated
@mrMoZ1 mrMoZ1 force-pushed the person/mzhivkov/library-issues branch from 05bac83 to af28805 Compare November 11, 2022 08:47
Copy link
Copy Markdown
Contributor

@antoniivanov antoniivanov left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Comment thread projects/vdk-core/src/vdk/internal/cli_entry.py
Comment thread projects/vdk-core/src/vdk/internal/cli_entry.py Outdated
mrMoZ1 added 15 commits November 11, 2022 16:50
Signed-off-by: mrMoZ1 <mzhivkov@vmware.com>
Signed-off-by: mrMoZ1 <mzhivkov@vmware.com>
Signed-off-by: mrMoZ1 <mzhivkov@vmware.com>
Signed-off-by: mrMoZ1 <mzhivkov@vmware.com>
Signed-off-by: mrMoZ1 <mzhivkov@vmware.com>
Signed-off-by: mrMoZ1 <mzhivkov@vmware.com>
Signed-off-by: mrMoZ1 <mzhivkov@vmware.com>
Signed-off-by: mrMoZ1 <mzhivkov@vmware.com>
Signed-off-by: mrMoZ1 <mzhivkov@vmware.com>
Signed-off-by: mrMoZ1 <mzhivkov@vmware.com>
Signed-off-by: mrMoZ1 <mzhivkov@vmware.com>
Signed-off-by: mrMoZ1 <mzhivkov@vmware.com>
Signed-off-by: mrMoZ1 <mzhivkov@vmware.com>
Signed-off-by: mrMoZ1 <mzhivkov@vmware.com>
Signed-off-by: mrMoZ1 <mzhivkov@vmware.com>
@mrMoZ1 mrMoZ1 force-pushed the person/mzhivkov/library-issues branch from af28805 to 5d6fd77 Compare November 11, 2022 14:50
Signed-off-by: mrMoZ1 <mzhivkov@vmware.com>
@mrMoZ1 mrMoZ1 enabled auto-merge (squash) November 11, 2022 14:55
@mrMoZ1 mrMoZ1 merged commit 725b040 into main Nov 11, 2022
@mrMoZ1 mrMoZ1 deleted the person/mzhivkov/library-issues branch November 11, 2022 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants