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.

Sign up for GitHub

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

Jump to bottom

chore: bazelified python services are tested for missing modules #12503

Merged
merged 2 commits into from Apr 21, 2022
Merged

chore: bazelified python services are tested for missing modules #12503

merged 2 commits into from Apr 21, 2022

Conversation

vktng
Copy link
Contributor

@vktng vktng commented Apr 20, 2022 •
edited

Signed-off-by: Krisztián Varga krisztian.varga@tngtech.com

Summary

The bazelified python services can be built successfully with bazel, but this does not mean that every necessary module is imported. The script provided here starts up these services and looks for ModuleNotFoundError in the logs. The script is added to the CI pipeline inside the "Bazel Build & Test" job.

Test Plan

To test all python services:
bazel/scripts/test_python_service_imports.sh
To test all python services inside a provided directory (recursively):
bazel/scripts/test_python_service_imports.sh lte/gateway/python/magma/
To test a single specified service:
bazel/scripts/test_python_service_imports.sh lte/gateway/python/magma/monitord:monitord

Additional Information

  • This change is backwards-breaking

@vktng vktng requested a review from mpfirrmann April 20, 2022 09:06
@vktng vktng self-assigned this Apr 20, 2022
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines. label Apr 20, 2022
@github-actions
Copy link
Contributor

Thanks for opening a PR! 💯

A couple initial guidelines

Howto

  • Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
  • Checks. All required CI checks must pass before merge.
  • Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

If this is your first Magma PR, also consider reading

@github-actions
Copy link
Contributor

github-actions bot commented Apr 20, 2022 •
edited

agw-workflow

     77 files     122 suites   6m 43s ⏱️
1 149 tests 1 140 ✔️ 9 💤 0 ❌
1 150 runs  1 141 ✔️ 9 💤 0 ❌

Results for commit 417ca6e.

♻️ This comment has been updated with latest results.

@vktng vktng force-pushed the bazel_service_import_test branch from 301e1ce to 031fc22 Compare April 20, 2022 11:54
@pull-request-size pull-request-size bot added size/L Denotes a Pull Request that changes 100-499 lines. and removed size/M Denotes a PR that changes 30-99 lines. labels Apr 20, 2022
@github-actions github-actions bot added component: ci All updates on CI (Jenkins/CircleCi/Github Action) component: orc8r Orchestrator-related issue labels Apr 20, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Apr 20, 2022 •
edited

Oops! Looks like you failed the DCO check. Be sure to sign all your commits.

Howto

♻️ Updated: ✅ The check is passing the DCO check after the last commit.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 20, 2022 •
edited

feg-workflow

    2 files  202 suites   38s ⏱️
370 tests 370 ✔️ 0 💤 0 ❌
384 runs  384 ✔️ 0 💤 0 ❌

Results for commit 6f5398a.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

cloud-workflow

    5 files  321 suites   2m 3s ⏱️
797 tests 797 ✔️ 0 💤 0 ❌

Results for commit f2dc96b.

@vktng vktng force-pushed the bazel_service_import_test branch from f2dc96b to 6f5398a Compare April 20, 2022 12:53
@github-actions github-actions bot added component: agw Access gateway-related issue and removed component: orc8r Orchestrator-related issue labels Apr 20, 2022
@vktng vktng force-pushed the bazel_service_import_test branch from 6f5398a to 031fc22 Compare April 20, 2022 13:44
@github-actions github-actions bot removed the component: agw Access gateway-related issue label Apr 20, 2022
@vktng vktng requested a review from nstng April 20, 2022 13:45
@vktng vktng marked this pull request as ready for review April 20, 2022 13:59
@vktng vktng requested a review from a team April 20, 2022 13:59
@themarwhal
Copy link
Member

Do you have screenshots of what failure cases might look like in CI? What type error a user might see, for example.

@vktng
Copy link
Contributor Author

vktng commented Apr 20, 2022

Do you have screenshots of what failure cases might look like in CI? What type error a user might see, for example.

Yes, here is a failed summary:
one_failing_import_error

You can find the complete log here: https://github.com/magma/magma/runs/6095247329?check_suite_focus=true

@themarwhal
Copy link
Member

Do you have screenshots of what failure cases might look like in CI? What type error a user might see, for example.

Yes, here is a failed summary: one_failing_import_error

You can find the complete log here: https://github.com/magma/magma/runs/6095247329?check_suite_focus=true

Looks great! :)

themarwhal
themarwhal approved these changes Apr 20, 2022
View reviewed changes
Copy link
Member

@themarwhal themarwhal left a comment

Choose a reason for hiding this comment

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

Looks great and this seems useful to have! I keep thinking that it would be great to have a comment bot that comments on failure to point to relevant Wiki page or the #bazel channel, but that is out of scope for this PR :D

LKreutzer
LKreutzer approved these changes Apr 20, 2022
View reviewed changes
Copy link
Contributor

@LKreutzer LKreutzer left a comment

Choose a reason for hiding this comment

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

Tested locally - lgtm.

@vktng
chore: bazelified python services are tested for missing modules
a3a1f54 
Signed-off-by: Krisztián Varga <krisztian.varga@tngtech.com>
@vktng
chore: test_python_service_imports.sh is included in the CI pipeline
417ca6e 
Signed-off-by: Krisztián Varga <krisztian.varga@tngtech.com>
@vktng vktng force-pushed the bazel_service_import_test branch from 031fc22 to 417ca6e Compare April 21, 2022 07:13
nstng
nstng approved these changes Apr 21, 2022
View reviewed changes
Copy link
Contributor

@nstng nstng left a comment

Choose a reason for hiding this comment

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

lgtm

@nstng nstng merged commit e336853 into magma:master Apr 21, 2022
@vktng
Copy link
Contributor Author

vktng commented Apr 21, 2022

Do you have screenshots of what failure cases might look like in CI? What type error a user might see, for example.

Yes, here is a failed summary:
one_failing_import_error

You can find the complete log here: https://github.com/magma/magma/runs/6095247329?check_suite_focus=true

Looks great and this seems useful to have! I keep thinking that it would be great to have a comment bot that comments on failure to point to relevant Wiki page or the #bazel channel, but that is out of scope for this PR :D

Yes, this is in progress and will be possible with the CI code ownership.

emakeev pushed a commit to emakeev/magma that referenced this pull request Aug 5, 2022
@vktng @emakeev
chore: bazelified python services are tested for missing modules (mag…
a06a5c4 
…ma#12503)

* chore: bazelified python services are tested for missing modules

Signed-off-by: Krisztián Varga <krisztian.varga@tngtech.com>

* chore: test_python_service_imports.sh is included in the CI pipeline

Signed-off-by: Krisztián Varga <krisztian.varga@tngtech.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LKreutzer LKreutzer LKreutzer approved these changes

@themarwhal themarwhal themarwhal approved these changes

@nstng nstng nstng approved these changes

@mpfirrmann mpfirrmann Awaiting requested review from mpfirrmann

Assignees

@vktng vktng

Labels
component: ci All updates on CI (Jenkins/CircleCi/Github Action) size/L Denotes a Pull Request that changes 100-499 lines.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@vktng @themarwhal @LKreutzer @nstng