-
Notifications
You must be signed in to change notification settings - Fork 0
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(launch): Provider for Run queues #22
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had one naming nit and I think we need to either make template_variables
and resource_config
required or have it so they don't error when not provided. Once I added those fields in my test it worked super smoothly!
baseURL := os.Getenv("WANDB_BASE_URL") | ||
apiKey := os.Getenv("WANDB_API_KEY") | ||
headers := http.Header{} | ||
headers.Set("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte("api:"+apiKey))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wandb-kc
Should we be using Bearer Token instead of Basic authorization here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be okay since we're not using OAuth tokens. I tend to think of username:password
with Basic Authorization but have seen instances where Basic is used to send tokens. I think Stripe does this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do the same in our SDK: https://github.com/wandb/wandb/blob/main/core/internal/api/send.go#L109
Initial implementation of a terraform provider for W&B Run Queues