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

Convert Windows registry queries to use winreg module #1780

Conversation

nrusch
Copy link
Contributor

@nrusch nrusch commented Jun 27, 2024

Python 3 adds a winreg module to the standard library to perform Windows registry queries using native APIs.

This change converts the rez registry queries to use the winreg API instead of executing cmd.exe and/or powershell and parsing their output.

Signed-off-by: Nathan Rusch <nathan.rusch@scanlinevfx.com>
@nrusch nrusch requested a review from a team as a code owner June 27, 2024 19:08
Copy link

linux-foundation-easycla bot commented Jun 27, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: nrusch / name: Nathan Rusch (bf72ceb)

@nrusch
Copy link
Contributor Author

nrusch commented Jun 27, 2024

This may also fix #1467

@JeanChristopheMorinPerso
Copy link
Member

Hi @nrusch , thanks for submitting this PR. We require all contributors to sign a CLA. You can look at the comment above from EasyCLA (#1780 (comment)) and follow the instructions. Let us know if you hit any problems.

@JeanChristopheMorinPerso JeanChristopheMorinPerso added the Blocked by CLA Waiting on CLA to be signed label Jun 28, 2024
@nrusch
Copy link
Contributor Author

nrusch commented Jun 28, 2024

Yes, I'm aware of the CLA requirement. We are working on getting our Netflix corporate CLA working, so I'm waiting for a follow-up from people higher up the chain.

@nrusch
Copy link
Contributor Author

nrusch commented Jul 1, 2024

OK, the CLA should be resolved now.

@JeanChristopheMorinPerso JeanChristopheMorinPerso removed the Blocked by CLA Waiting on CLA to be signed label Jul 1, 2024
@samson-jerome
Copy link
Contributor

Hi @nrusch thanks for this PR, we're also really interested in having this merged !
I was having issues with how powershell plugin queries the registry to get the standard_system_paths. It is much slower than what is currently done in windows.get_syspaths_from_registry for an identical result (on my end).

Now using winreg seems an even better option so here's a little bump for the tsc members :)

@JeanChristopheMorinPerso JeanChristopheMorinPerso added this to the Next milestone Sep 19, 2024
@JeanChristopheMorinPerso
Copy link
Member

Thank you @samson-jerome for the great feedback and trying it out! We are planning on merging this before our next release, which should hopefully happen in the next 2-3 weeks.

Copy link
Member

@JeanChristopheMorinPerso JeanChristopheMorinPerso left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you!

@JeanChristopheMorinPerso JeanChristopheMorinPerso merged commit eb0e920 into AcademySoftwareFoundation:main Oct 13, 2024
3 checks 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.

3 participants