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

feat(contrib/qxip/hash): add sha1, md5, base64, hmac #5371

Merged
merged 16 commits into from
Jan 24, 2023

Conversation

lmangani
Copy link
Contributor

@lmangani lmangani commented Jan 21, 2023

This PR extends the contrib/qxip/hash package with additional functions for sha1, base64, md5, hmac

Note: The primary goal of this extension is powering flux native extensions dealing with auth token calculation etc. (ie: s3 client we're working on) without having to use or import additional go code.

  • Updated Documentation
  • Extended Test Converage

@lmangani lmangani marked this pull request as ready for review January 21, 2023 21:04
@lmangani lmangani requested review from a team as code owners January 21, 2023 21:04
@lmangani lmangani requested review from gavincabbage and sanderson and removed request for a team January 21, 2023 21:04
@lmangani lmangani marked this pull request as draft January 22, 2023 22:17
@lmangani lmangani marked this pull request as ready for review January 23, 2023 00:22
@lmangani lmangani changed the title feat(contrib/qxip/hash): add sha1, md5, base64 feat(contrib/qxip/hash): add sha1, md5, base64, hmac Jan 23, 2023
Copy link
Contributor

@sanderson sanderson left a comment

Choose a reason for hiding this comment

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

Thanks @lmangani! A few minor suggestions on the docs, but this looks great.

stdlib/contrib/qxip/hash/hash.flux Outdated Show resolved Hide resolved
stdlib/contrib/qxip/hash/hash.flux Outdated Show resolved Hide resolved
stdlib/contrib/qxip/hash/hash.flux Outdated Show resolved Hide resolved
stdlib/contrib/qxip/hash/hash.flux Outdated Show resolved Hide resolved
stdlib/contrib/qxip/hash/hash.flux Outdated Show resolved Hide resolved
stdlib/contrib/qxip/hash/hash.flux Outdated Show resolved Hide resolved
stdlib/contrib/qxip/hash/hash.flux Outdated Show resolved Hide resolved
Co-authored-by: Scott Anderson <sanderson@users.noreply.github.com>
@lmangani lmangani requested a review from a team as a code owner January 23, 2023 18:27
lmangani and others added 7 commits January 23, 2023 19:27
Co-authored-by: Scott Anderson <sanderson@users.noreply.github.com>
Co-authored-by: Scott Anderson <sanderson@users.noreply.github.com>
Co-authored-by: Scott Anderson <sanderson@users.noreply.github.com>
Co-authored-by: Scott Anderson <sanderson@users.noreply.github.com>
Co-authored-by: Scott Anderson <sanderson@users.noreply.github.com>
Co-authored-by: Scott Anderson <sanderson@users.noreply.github.com>
@lmangani
Copy link
Contributor Author

Thank you so much @sanderson! Next attempt in line is an S3 HTTP client for flux 100% made in flux 🤞

@sanderson sanderson merged commit 1eba4ff into influxdata:master Jan 24, 2023
@lmangani
Copy link
Contributor Author

lmangani commented Jan 24, 2023

@sanderson @gavincabbage I'm going into paranoid mode temporarily: building with the latest flux master causes the following error, which seems related to this PR. I'm not sure if this is just about how I'm building my binary, but I'd rather be safe than sorry:

panic: missing builtin values [sha1 b64 md5 hmac], extra builtin values []

goroutine 1 [running]:
github.com/influxdata/flux/runtime.FinalizeBuiltIns(...)
        /home/runner/go/pkg/mod/github.com/influxdata/flux@v0.192.0/runtime/global.go:97
github.com/influxdata/flux/fluxinit.FluxInit(...)
        /home/runner/go/pkg/mod/github.com/influxdata/flux@v0.192.0/fluxinit/init.go:23
github.com/influxdata/flux/fluxinit/static.init.0()
        /home/runner/go/pkg/mod/github.com/influxdata/flux@v0.192.0/fluxinit/static/static.go:10 +0x3f

Did I miss something in the new code? The previous hash contrib was working fine. Apologies in advance!

@sanderson
Copy link
Contributor

@lmangani I think you need to regenerate the stdlib before you build.

> go generate ./stdlib ./libflux/go/libflux
> make generate

@lmangani
Copy link
Contributor Author

I think this could be caused by me building using the master for the go library and a release (v0.192.0) as the library (or vice versa) which does not include yet the changes? As long as this is a build issue only on my side, all is well... Thanks @sanderson

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

3 participants