yubihsm.rs icon indicating copy to clipboard operation
yubihsm.rs copied to clipboard

Make connectable and connection public

Open mikroskeem opened this issue 1 year ago • 8 comments

This allows to implement new connector backends externally (e.g. HTTPS or UDS) by implementing Connectable and Connection traits.

mikroskeem avatar Aug 27 '23 00:08 mikroskeem

reqwest has several hundred dependencies which is why we don't use it, however it might make more sense to add a native HTTPS connector using a more lightweight client like ureq.

I'm not familiar with UDS... it it something you actually intend to use?

tony-iqlusion avatar Aug 27 '23 19:08 tony-iqlusion

While implementing https connector in the library would cover one of the use-cases that yubihsm-connector supports, I would still love to proceed with this approach as well.

Now if I bring UDS as an example, it's not something what yubihsm-connector officially supports at first place, and is rather specific to my organization - therefore it's not worth upstreaming & maintaining here.

I believe this change could be useful for others who want to do non-standard communication and don't want to maintain a fork of this library.

mikroskeem avatar Aug 27 '23 21:08 mikroskeem

Bump, is this PR still acceptable?

mikroskeem avatar Jan 19 '24 00:01 mikroskeem

It's out-of-date with the base branch.

And... I guess, I don't understand your use case and it's more API surface to keep stable instead of being able to refactor, but I guess the counterpoint is we haven't touched it in awhile so it is defacto stable.

tony-iqlusion avatar Jan 19 '24 12:01 tony-iqlusion

I don't know how I can explain the use-case simpler than I did in PR description... perhaps that to keep this library small and not pull in heavy dependencies like reqwest (the gist I referenced is an example that can be achieved once this PR is merged), yet library consumers being able to introduce their own transport implementations without needing to maintain their own forks.

Reason why I asked if this PR is acceptable, is to decide if I'll bother with rebasing it, or just close it.

mikroskeem avatar Jan 19 '24 18:01 mikroskeem

I guess I can merge it if you update it

tony-iqlusion avatar Jan 19 '24 18:01 tony-iqlusion

Rebase done

mikroskeem avatar Jan 20 '24 14:01 mikroskeem