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

drop the need for "shell magic" in dkron run_async with a 'proxy' command #31

Merged
merged 1 commit into from
Mar 23, 2023

Conversation

fopina
Copy link
Contributor

@fopina fopina commented Mar 19, 2023

Make use of a proxy command to call run_async commands with call_command() dropping the need for shell magic to handle escaping/invalid characters, dashes for flags, etc.

This is mostly backward compatible except for the cases where callers implemented hacks/workarounds to escape the characters themselves... But that should be easy to fix and the proper approach as, again, those were workarounds/hacks...

PR tested with the new unit test and also, for integration, manually tested with

$ ./manage.py shell -c 'from dkron.utils import run_async; run_async("shell", command="import time;print(1);time.sleep(2);print(2)")'

(after starting dkron server run_dkron)

This PR fixes #15

@fopina fopina requested a review from a team as a code owner March 19, 2023 19:04
@fopina fopina requested review from tcppb and fpintoppb March 19, 2023 19:04
@fopina fopina marked this pull request as draft March 19, 2023 19:04
@github-advanced-security
Copy link

You have successfully added a new SonarCloud configuration ``. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

@fopina fopina force-pushed the fix/improve_run_async_command branch 2 times, most recently from 9db5fd1 to 3acb48b Compare March 20, 2023 23:26
@fopina fopina marked this pull request as ready for review March 20, 2023 23:26
@fopina fopina force-pushed the fix/improve_run_async_command branch from 3acb48b to b3eed58 Compare March 20, 2023 23:29
@fopina fopina requested a review from gsilvapt March 20, 2023 23:31
@codecov-commenter
Copy link

codecov-commenter commented Mar 20, 2023

Codecov Report

Patch coverage: 96.55% and project coverage change: +1.14 🎉

Comparison is base (21c9383) 88.49% compared to head (7bcc4f2) 89.64%.

❗ Current head 7bcc4f2 differs from pull request most recent head 24e2db5. Consider uploading reports for the commit 24e2db5 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@             Coverage Diff             @@
##           develop      #31      +/-   ##
===========================================
+ Coverage    88.49%   89.64%   +1.14%     
===========================================
  Files           14       15       +1     
  Lines          513      541      +28     
===========================================
+ Hits           454      485      +31     
+ Misses          59       56       -3     
Impacted Files Coverage Δ
dkron/migrations/0001_initial_20210729.py 100.00% <ø> (ø)
dkron/migrations/0002_job_retries.py 100.00% <ø> (ø)
dkron/utils.py 86.84% <94.59%> (+1.69%) ⬆️
dkron/management/commands/run_dkron.py 91.22% <100.00%> (+2.51%) ⬆️
...ron/management/commands/run_dkron_async_command.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@fopina fopina mentioned this pull request Mar 22, 2023
@fopina fopina force-pushed the fix/improve_run_async_command branch from b3eed58 to 8743b89 Compare March 23, 2023 11:55
@fopina
Copy link
Contributor Author

fopina commented Mar 23, 2023

develop rebased to resolve conflicts 👌

@fopina fopina force-pushed the fix/improve_run_async_command branch from 7bcc4f2 to 24e2db5 Compare March 23, 2023 12:19
@sonarcloud
Copy link

sonarcloud bot commented Mar 23, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@fopina fopina merged commit 886d1a3 into develop Mar 23, 2023
@DDuarte DDuarte deleted the fix/improve_run_async_command branch February 16, 2024 13:36
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.

improve parameter handling in run_async
3 participants