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]: add helper utils #29

Merged
merged 3 commits into from
Nov 8, 2024
Merged

[FIX]: add helper utils #29

merged 3 commits into from
Nov 8, 2024

Conversation

tpegl
Copy link
Contributor

@tpegl tpegl commented Oct 17, 2024

Addresses #4 by moving output functions and variables to utils.py and importing from it in modules.

  • Create some new functions using the colorama variables to make output more consistent and easier to use
  • Move some imports around based on pylint recommendation

This PR moves a lot and touches almost every file so there's no rush on checking it. Please take as long as you need and if it's too much (or just not how you'd like it done), please let me know 🙏 thanks for letting me contribute more!

Vapourisation added 2 commits October 17, 2024 17:13
Add extra catch-all arg to utils print functions and pass to internal print
@6abd
Copy link
Owner

6abd commented Nov 1, 2024

Sorry for the late response, will check it out this weekend!

@6abd
Copy link
Owner

6abd commented Nov 3, 2024

Everything seems good, but I get the following warning after running a command:

/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/multiprocessing/resource_tracker.py:224: UserWarning: resource_tracker: There appear to be 1 leaked semaphore objects to clean up at shutdown warnings.warn('resource_tracker: There appear to be %d '

Does this only happen on my machine?

@tpegl
Copy link
Contributor Author

tpegl commented Nov 5, 2024

What OS and what command?

I don't see it when I run ovpn and config but I'm on EndeavourOS. I'll check on Windows too and see if I can get some MacOS to try on

@6abd
Copy link
Owner

6abd commented Nov 5, 2024

MacOS, apicon and falcon when I tested them, but I think it's on every command for me.

@tpegl
Copy link
Contributor Author

tpegl commented Nov 6, 2024

Reproduced it on a Mac, Python 3.11.4. I think it might be Mac specific but I'll need to investigate it. It's just a warning at least but it appears to be Mac specific 🤔 I'll investigate. Oddly it is most likely from my previous PR, that was the only place I used multiprocessing.

@tpegl
Copy link
Contributor Author

tpegl commented Nov 6, 2024

I haven't been able to find any useful threads on this specific issue but I had a suspicion that the os._exit calls might be causing it so swapped them out to sys.exit and it removed the warning. Do you remember why os._exit was used instead of sys.exit?

@6abd
Copy link
Owner

6abd commented Nov 6, 2024

I honestly couldn't tell you, but that seems to have been the issue. Could you update your changes for that fix? I can merge it once that's done. Thanks!

@tpegl
Copy link
Contributor Author

tpegl commented Nov 7, 2024

Absolutely, I'll update it now!

@6abd 6abd merged commit c1d093e into 6abd:main Nov 8, 2024
1 check passed
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