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

Implement WindowsCredentialsDatabaseSelector #3661

Merged
merged 34 commits into from
Sep 12, 2023

Conversation

shreyamalviya
Copy link
Contributor

@shreyamalviya shreyamalviya commented Sep 6, 2023

What does this PR do?

Adds the WindowsCredentialsDatabaseSelector component for #3426

PR Checklist

  • Have you added an explanation of what your changes do and why you'd like to include them?
  • Is the TravisCI build passing?
  • Was the CHANGELOG.md updated to reflect the changes?
  • Was the documentation framework updated to reflect the changes?
  • Have you checked that you haven't introduced any duplicate code?

Testing Checklist

  • Added relevant unit tests?
  • Do all unit tests pass?
  • Do all end-to-end tests pass?
  • Any other testing performed?

    Tested by running the selector and verifying the result

  • If applicable, add screenshots or log transcripts of the feature working

@shreyamalviya shreyamalviya force-pushed the 3426-windows-credentials-database-selector branch from 8067246 to 8fa926e Compare September 6, 2023 13:58
@shreyamalviya shreyamalviya marked this pull request as ready for review September 7, 2023 10:52
@cakekoa cakekoa force-pushed the 3426-windows-credentials-database-selector branch 2 times, most recently from a177bc4 to 9171a1a Compare September 7, 2023 21:33
@shreyamalviya shreyamalviya force-pushed the 3426-windows-credentials-database-selector branch from 9171a1a to dade8cd Compare September 8, 2023 07:32
@shreyamalviya shreyamalviya changed the base branch from develop to 3426-prepare-for-testing September 8, 2023 07:34
Base automatically changed from 3426-prepare-for-testing to develop September 8, 2023 11:47
@shreyamalviya shreyamalviya force-pushed the 3426-windows-credentials-database-selector branch from 36ce415 to 26aad7e Compare September 8, 2023 11:54
@mssalvatore
Copy link
Collaborator

Missing unit tests.

@mssalvatore
Copy link
Collaborator

I think all the PurePaths need to just be Paths.

Pure paths are useful in some special cases; for example:

  • If you want to manipulate Windows paths on a Unix machine (or vice versa). You cannot instantiate a WindowsPath when running on Unix, but you can instantiate PureWindowsPath.

  • You want to make sure that your code only manipulates paths without actually accessing the OS. In this case, instantiating one of the pure classes may be useful since those simply don’t have any OS-accessing operations.

https://docs.python.org/3/library/pathlib.html

@cakekoa cakekoa force-pushed the 3426-windows-credentials-database-selector branch 3 times, most recently from a817ceb to d6555c1 Compare September 11, 2023 19:55
@shreyamalviya shreyamalviya force-pushed the 3426-windows-credentials-database-selector branch from d6555c1 to 87662ba Compare September 12, 2023 08:55
@shreyamalviya shreyamalviya force-pushed the 3426-windows-credentials-database-selector branch from 87662ba to 9f745a6 Compare September 12, 2023 11:24
@mssalvatore mssalvatore merged commit 6a6abbe into develop Sep 12, 2023
@mssalvatore mssalvatore deleted the 3426-windows-credentials-database-selector branch September 12, 2023 17:04
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