Skip to content

optimistic wallet identity added as annotation to submitted bacalhau jobs #601

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 17, 2023
Merged

optimistic wallet identity added as annotation to submitted bacalhau jobs #601

merged 2 commits into from
Aug 17, 2023

Conversation

acashmoney
Copy link
Contributor

Annotations are added in the format userId=<optimistic wallet identity>

@acashmoney
plex create w/ autoRun flag set to true and plex run default add opti…
f8552a2 
…mistic wallet identity annotation to bacalhau jobs
@acashmoney acashmoney requested a review from hevans66 August 15, 2023 07:23
@acashmoney acashmoney linked an issue Aug 15, 2023 that may be closed by this pull request
[LAB-500] add optimistic wallet identity to bacalhau job annotation #588
Closed
@linear
Copy link

linear bot commented Aug 15, 2023

LAB-500 add optimistic wallet identity to bacalhau job annotation

Related to LAB-341. This will let us associate users' wallet addresses with bacalhau job IDs for plex get jobs

@vercel
Copy link

vercel bot commented Aug 15, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ❌ Failed (Inspect) Aug 17, 2023 11:07pm

@acashmoney acashmoney temporarily deployed to ci August 15, 2023 07:23 — with GitHub Actions Inactive
@acashmoney
Copy link
Contributor Author

@hevans66 looking for similar feedback to #600 with an additional wrinkle. Would you prefer this form of annotation (userId=...) so it's more clear when updating the metabase queries?

@acashmoney
Copy link
Contributor Author

Update: this needs to be built for plex init as well

hevans66
hevans66 approved these changes Aug 15, 2023
View reviewed changes
Copy link
Contributor

@hevans66 hevans66 left a comment

Choose a reason for hiding this comment

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

This works for me. My plan will be to pull the userId out and save it as a column in the DB, so as long as I can find it in the JSON, it will be all good.

cmd/create.go
if err != nil {
fmt.Println("Error:", err)
os.Exit(1)
}

if autoRun && userID != "" {
*annotationsForAutoRun = append(*annotationsForAutoRun, fmt.Sprintf("userId=%s", userID))
Copy link
Contributor

Choose a reason for hiding this comment

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

What characters are possible values in this wallet id? It would be nice to pick a separator that won't occur in the given user id. Or at least be aware that it can exist for parsing down the line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hevans66 userId can only point to an EVM wallet address. We have a helper validator function with some regex that could be helpful for parsing on the metabase side.

plex/internal/web3/wallet.go

Lines 7 to 11 in f8552a2

func IsValidEthereumAddress(address string) bool {
pattern := `^0x[0-9a-fA-F]{40}$`
match, _ := regexp.MatchString(pattern, address)
return match
}

@acashmoney acashmoney temporarily deployed to ci August 17, 2023 23:07 — with GitHub Actions Inactive
@acashmoney acashmoney merged commit 4604cce into main Aug 17, 2023
@acashmoney acashmoney deleted the 588-lab-500-add-optimistic-wallet-identity-to-bacalhau-job-annotation branch August 17, 2023 23:12
@what-the-diff
Copy link

what-the-diff bot commented Aug 21, 2023

PR Summary

  • Enhancement to CreateIO Function
    The function CreateIO in the cmd/create.go module has been enhanced to now return userID as one of its output values. This added information makes it easier to track and manage user sessions.

  • Update in Annotations for Auto Run
    The code segment in cmd/create.go responsible for generating annotations for auto run has been adjusted. It now includes the userID, which extends the traceability of each auto-run task to the initiating user.

  • IO File Parsing Updates
    Changes have been made to the cmd/run.go and internal/ipwl/io.go modules for more efficient handling of IO JSON files. These modules now have the ability to extract the userID and append it to the list of annotations.

  • New Functions for User ID Annotation
    Two new functions ContainsUserIdAnnotation and ExtractUserIDFromIOJson have been introduced in internal/ipwl/io.go. These functions aim to streamline the process of extracting and utilizing userID information present in the IO data.

  • Minor Refinements
    There have also been minor changes to improve clarity, including adjustments to print statements and variable naming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@hevans66 hevans66 hevans66 approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[LAB-500] add optimistic wallet identity to bacalhau job annotation
2 participants
@acashmoney @hevans66