feat(admissions): implement encoding of admissions extension #11892
Conversation
hoefling
changed the title
src/rust/src/x509/extensions.rs
Outdated
| py: pyo3::Python<'_>, | ||
| ka_str: &'a cryptography_keepalive::KeepAlive<pyo3::pybacked::PyBackedStr>, | ||
| py_naming_authority: &pyo3::Bound<'a, pyo3::PyAny>, | ||
| ) -> Result<extensions::NamingAuthority<'a>, CryptographyError> { |
There was a problem hiding this comment.
You can use CryptographyResult<extensions::NamingAuthority<'a>> which is marginally shorter
There was a problem hiding this comment.
Duly noted :-) also switched to CryptographyResult for consistency in other introduced functions as well.
src/rust/src/x509/extensions.rs
Outdated
| py_naming_authority: &pyo3::Bound<'a, pyo3::PyAny>, | ||
| ) -> Result<extensions::NamingAuthority<'a>, CryptographyError> { | ||
| let py_oid = py_naming_authority.getattr(pyo3::intern!(py, "id"))?; | ||
| let id = if py_oid.is_truthy()? { |
There was a problem hiding this comment.
Prefer is_none(), it better expresses what we want, and is also a smidge faster.
There was a problem hiding this comment.
This is a really great hint, thank you! Indeed using it makes sense in all occurences, and by doing that I even uncovered an error I slipped into the codebase in #11881 - the professionOIDs field is optional, but I missed that. Will amend this in a separate PR.
There was a problem hiding this comment.
@alex what I forgot to ask: what do you prefer more?
let val = if !py_val.is_none() {
Some(...)
} else {
None
};or
let val = if py_val.is_none() {
None
} else {
Some(...)
};There was a problem hiding this comment.
Whichever you prefer, I'm sure we're not consistent
hoefling
force-pushed
the
feature/extension/admissions/encode
branch
2 times, most recently
from
03ca018
to
280424e
Compare
hoefling
force-pushed
the
feature/extension/admissions/encode
branch
from
280424e
to
c6c825b
Compare
hoefling
force-pushed
the
feature/extension/admissions/encode
branch
from
c6c825b
to
b403042
Compare
Signed-off-by: oleg.hoefling <[email protected]>
Signed-off-by: oleg.hoefling <[email protected]>
Signed-off-by: oleg.hoefling <[email protected]>
Signed-off-by: oleg.hoefling <[email protected]>
Signed-off-by: oleg.hoefling <[email protected]>
…rity with none values Signed-off-by: oleg.hoefling <[email protected]>
Signed-off-by: oleg.hoefling <[email protected]>
Signed-off-by: oleg.hoefling <[email protected]>
… in a separate pr Signed-off-by: oleg.hoefling <[email protected]>
… to a printablestring Signed-off-by: oleg.hoefling <[email protected]>
Signed-off-by: oleg.hoefling <[email protected]>
hoefling
force-pushed
the
feature/extension/admissions/encode
branch
from
b403042
to
526e083
Compare
This is the fifth PR for #11875 which adds encoding of the
Admissionsextension object. Please review with caution - this is the part I am least confident in, and will be grateful for all improvement suggestions. Please also bear with my Rust coding skills - for this PR, I mostly learned from the existing codebase and PyO3 docs.