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

Avoid running command line scripts twice on every invocation #2878

Merged
merged 2 commits into from
Mar 20, 2024

Conversation

lunkwill42
Copy link
Member

Fixes #2877

Instead of using a "double" shim to install these executable scripts,
install their respective modules directly.  The existing shim scripts
would run the `main()` method as a side-effect of the module being
imported, which was a bad idea.  With the new script installation
method, these commands would essentially run twice on every
invocation.
This adds an import-time guard to ensure that `main()` isn't run twice
when command is run through the install-time shim.

Opted to leave this script in nav.bin, since it has pre-main setup
that isn't in the imported module.
@lunkwill42 lunkwill42 requested a review from hmpf March 20, 2024 10:13
@lunkwill42 lunkwill42 self-assigned this Mar 20, 2024
@lunkwill42 lunkwill42 added the bug label Mar 20, 2024
Copy link

codecov bot commented Mar 20, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 56.60%. Comparing base (7f0f71c) to head (65996fa).
Report is 5 commits behind head on master.

Files Patch % Lines
python/nav/bin/navtopology.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2878      +/-   ##
==========================================
- Coverage   56.64%   56.60%   -0.05%     
==========================================
  Files         603      601       -2     
  Lines       43984    43981       -3     
==========================================
- Hits        24916    24896      -20     
- Misses      19068    19085      +17     

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

Copy link

Test results

     12 files       12 suites   11m 18s ⏱️
3 320 tests 3 320 ✔️ 0 💤 0
9 435 runs  9 435 ✔️ 0 💤 0

Results for commit 65996fa.

pyproject.toml Show resolved Hide resolved
hmpf
hmpf previously approved these changes Mar 20, 2024
@lunkwill42 lunkwill42 changed the base branch from master to 5.9.x March 20, 2024 11:07
@lunkwill42 lunkwill42 dismissed hmpf’s stale review March 20, 2024 11:07

The base branch was changed.

@hmpf hmpf self-requested a review March 20, 2024 11:23
@lunkwill42 lunkwill42 merged commit 8fb0738 into Uninett:5.9.x Mar 20, 2024
9 of 11 checks passed
@lunkwill42 lunkwill42 deleted the bugfix/dont-run-scripts-twice branch March 20, 2024 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Several NAV scripts run twice on every invocation
2 participants