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

Add documentation for cred store common values #157

Merged
merged 1 commit into from
Oct 3, 2018

Conversation

simo5
Copy link
Contributor

@simo5 simo5 commented Sep 25, 2018

Add docs to explain what cred store values can be used and how

Copy link
Member

@DirectXMan12 DirectXMan12 left a comment

Choose a reason for hiding this comment

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

generally looks good, but couple of comments inline

docs/source/conf.py Show resolved Hide resolved
Common Values for Credentials Store Extensions
==============================================

.. py:module:: gssapi.raw
Copy link
Member

Choose a reason for hiding this comment

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

this isn't the gssapi.raw module

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what is it ?
sphinx complains if there is no module

@@ -28,6 +28,7 @@ straight into the :doc:`high-level API documentation <gssapi>`.

gssapi.rst
gssapi.raw.rst
credstore.rst
Copy link
Member

Choose a reason for hiding this comment

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

I'd add an extra level of indirection in the ToC, like we do for tutorials. So maybe Implementation-Specific Tips and Documentation, and then list the Cred Store stuff under that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

:func:`gssapi.raw.ext_cred_store.add_cred_from` functions, to indicate a
custom location for a keytab containing client keys.
It is not used in the context of calls used to store credentials.
The value is a string in the form "type:residual" where type can be any
Copy link
Member

Choose a reason for hiding this comment

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

the word residual is a bit unclear here. Maybe something like

type:config, where config is some type-specific configuration. For instance, for the FILE type, config is a path, like FILE:/tmp/my.keytab

Copy link
Contributor Author

Choose a reason for hiding this comment

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

residual is the technical name as found in MIT docs, at most I can use "path" if you think residual is to be avoided.

Copy link
Member

Choose a reason for hiding this comment

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

maybe put in in parens or something. It's non-obvious to most people that haven't already read the MIT docs, but it's good to have it there for people who have.

@simo5
Copy link
Contributor Author

simo5 commented Sep 26, 2018

@DirectXMan12 PTAL

@simo5 simo5 dismissed DirectXMan12’s stale review September 26, 2018 19:37

should all be addressed now

@simo5
Copy link
Contributor Author

simo5 commented Oct 2, 2018

@DirectXMan12 ping ?

@DirectXMan12
Copy link
Member

responded to the type:residual comment, otherwise looks good

Signed-off-by: Simo Sorce <simo@redhat.com>
@DirectXMan12 DirectXMan12 merged commit 4a2736e into pythongssapi:master Oct 3, 2018
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.

None yet

2 participants