Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add krb5_cc_retrieve_cred() and krb5_cc_remove_cred() #23

Merged

Conversation

steffen-kiess
Copy link
Contributor

No description provided.

@steffen-kiess steffen-kiess force-pushed the add-retrieve_cred-remove_cred branch from dd3deda to f51bf86 Compare December 15, 2022 11:57
@steffen-kiess
Copy link
Contributor Author

KRB5_TC_SUPPORTED_KTYPES is not available on heimdal. I've updated the PR to make CredentialsRetrieveFlags.supported_ktypes available only if KRB5_TC_SUPPORTED_KTYPES is available at compile time.

Copy link
Owner

@jborean93 jborean93 left a comment

Choose a reason for hiding this comment

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

I'll have to look into the packaging side to figure out why it's failing. Any chance on adding actual tests for these new functions?

@jborean93
Copy link
Owner

#24 should fix up the CI errors, it seems like Ubuntu 22.04 ships with a version of pip which is incompatible with how we test things. That PR moves the project structure back to using the setup.cfg style project definition which improves compatibility. If you rebase your PR it should pick up those changes for the next run.

Looking at the macOS errors it looks like you need to define the enum in the _ccache.pyi file for mypy to be happy. https://github.com/jborean93/pykrb5/blob/main/src/krb5/_principal.pyi#L9-L24 is how I did it for other enums.

@steffen-kiess
Copy link
Contributor Author

Any chance on adding actual tests for these new functions?

I can try to do that.

That PR moves the project structure back to using the setup.cfg style project definition which improves compatibility. If you rebase your PR it should pick up those changes for the next run.

Ok, I'll do that.

@steffen-kiess steffen-kiess force-pushed the add-retrieve_cred-remove_cred branch 4 times, most recently from 459c498 to d7ec7cf Compare December 16, 2022 17:10
@steffen-kiess steffen-kiess force-pushed the add-retrieve_cred-remove_cred branch from d7ec7cf to 5290637 Compare December 16, 2022 17:23
@steffen-kiess
Copy link
Contributor Author

I've added a test, and the CI now seems to run through.

@jborean93
Copy link
Owner

Thanks for working on this and filling out the tests and typing stuff, appreciate the work.

@jborean93 jborean93 merged commit 57dbb0e into jborean93:main Dec 16, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants