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

💎 Support digest_function in GrpcStore #1325

Open
bclark8923 opened this issue Sep 5, 2024 · 12 comments · May be fixed by #1548
Open

💎 Support digest_function in GrpcStore #1325

bclark8923 opened this issue Sep 5, 2024 · 12 comments · May be fixed by #1548
Labels
💎 Bounty bug Something isn't working good first issue Good for newcomers

Comments

@bclark8923
Copy link
Member

When using an alternative hash function to the default (sha256), GRPC store doesn't receive which hash function to use.

@bclark8923 bclark8923 added bug Something isn't working good first issue Good for newcomers labels Sep 5, 2024
@bclark8923 bclark8923 changed the title Support digest_function in GRPC Store Support digest_function in GrpcStore Sep 6, 2024
@MarcusSorealheis
Copy link
Collaborator

If you are looking to solve this issue, look into resource_info.rs. It predates our support
for Blake3.

@aleksdmladenovic
Copy link
Contributor

I will work on this issue. cc: @MarcusSorealheis, @allada, @bclark8923.

@aleksdmladenovic
Copy link
Contributor

Hi, @bclark8923.

I looked over details of this issue and I have some things unclear about this issue.

FYI, from this PR, we now detect digest function automatically from the request.

And there is code for GrpcStore receiving digest_function from ActiveOriginContext in grpc_store.rs.

digest_function: ActiveOriginContext::get_value(&ACTIVE_HASHER_FUNC)
.err_tip(|| "In get_action_from_store")?
.map_or_else(default_digest_hasher_func, |v| *v)
.proto_digest_func()
.into(),

digest_function: ActiveOriginContext::get_value(&ACTIVE_HASHER_FUNC)
.err_tip(|| "In get_action_from_store")?
.map_or_else(default_digest_hasher_func, |v| *v)
.proto_digest_func()
.into(),

digest_function: ActiveOriginContext::get_value(&ACTIVE_HASHER_FUNC)
.err_tip(|| "In GrpcStore::has_with_results")?
.map_or_else(default_digest_hasher_func, |v| *v)
.proto_digest_func()
.into(),

And just fyi, while I was trying to evaluate this issue, I found out that there were no test files for testing GrpcStore feature.

Did we omit this for specific purpose or just missing and need to be added later?

And I hope you to make it clear how you caught this issue and how you tested.

Thanks.

cc: @allada, @MarcusSorealheis

@MarcusSorealheis
Copy link
Collaborator

@aleksdmladenovic if there is no test we need to write one.

@MarcusSorealheis
Copy link
Collaborator

/bounty $1000

Copy link

algora-pbc bot commented Nov 20, 2024

💎 $1,000 bounty • TraceMachina

Steps to solve:

  1. Start working: Comment /attempt #1325 with your implementation plan
  2. Submit work: Create a pull request including /claim #1325 in the PR body to claim the bounty
  3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

Thank you for contributing to TraceMachina/nativelink!

Add a bountyShare on socials

Attempt Started (GMT+0) Solution
🟢 @asr2003 Nov 21, 2024, 5:20:15 AM #1548

@MarcusSorealheis
Copy link
Collaborator

@asr2003
Copy link

asr2003 commented Nov 21, 2024

@MarcusSorealheis This is Sahithi from Algora expert community! I would like to connect and tackle this issue and I have gone through the program guidelines attached above. Well, I need to discuss my approach before I design document what's I am thinking of in Slack. Can we continue discussion in a draft design PR or in Slack. Happy to contribute to nativelink :)

Can I get this issue assigned?

/attempt #1325

Algora profile Completed bounties Tech Active attempts Options
@asr2003 9 bounties from 4 projects
Scala, Rust,
Go & more
Cancel attempt

@MarcusSorealheis
Copy link
Collaborator

@asr2003 you need to start with. Design document per the details in the gist above.

@aaronmondal aaronmondal pinned this issue Nov 21, 2024
@asr2003
Copy link

asr2003 commented Nov 21, 2024

@MarcusSorealheis Here's my draft of design doc for this issue.
https://docs.google.com/document/d/1VI3OwfBNgIeF_gJ38yILoH-WgJIrVwOz/

I am happy to incorporate any suggestions and feedback :)

@MarcusSorealheis
Copy link
Collaborator

MarcusSorealheis commented Nov 21, 2024 via email

@aaronmondal aaronmondal changed the title Support digest_function in GrpcStore 💎 Support digest_function in GrpcStore Dec 13, 2024
@asr2003 asr2003 linked a pull request Dec 17, 2024 that will close this issue
4 tasks
Copy link

algora-pbc bot commented Dec 17, 2024

💡 @asr2003 submitted a pull request that claims the bounty. You can visit your bounty board to reward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💎 Bounty bug Something isn't working good first issue Good for newcomers
Projects
None yet
4 participants