Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: don't store token when certificates are configured#192

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

fioan89
Copy link
Collaborator

This was a regression for the custom flows that required authentication via certificates, the http client and the coder cli were properly initialized but afterward the token was still required for storing.

Note: the issue was hard to catch early on because the official coder cli is not supporting auth. via certificates.

This was a regression for the custom flows that required authentication via certificates,the http client and the coder cli were properly initialized but afterward the token wasstill required for storing.Note: the issue was hard to catch early on because the official coder cli is not supporting auth. via certificates.
Poll jobs now have a random friendly name that changes each timea new job is created, and the logging for when the jobs are createdor cancelled is also improved. The idea behind this commit is to improvedebugging sessions on client logs by:- having clear log lines saying when a job is created/destructed- easy to remember and cross-match job names (instead of cryptic java reference values)
@fioan89fioan89 changed the titleffix: don't store token when certificates are configuredfix: don't store token when certificates are configuredSep 12, 2025
Seems like we can't properly get the coroutine name from a Job.The name of the coroutine is not super friendly but still workable withthe new logs.
Copy link
Member

@code-ashercode-asher left a comment

Choose a reason for hiding this comment

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

Looks good to me but was this causing a bug or something? Looks like it would just store a blank string in the secrets store which seems like it would essentially just be a no-op.

@fioan89
Copy link
CollaboratorAuthor

Looks good to me but was this causing a bug or something? Looks like it would just store a blank string in the secrets store which seems like it would essentially just be a no-op.

Yeap, it raised an error. Seems like the underlying store will not allow nu/empty values.
image

@code-asher
Copy link
Member

code-asher commentedSep 13, 2025
edited
Loading

Trippy! Ohhh and I thought a blank value would callstore.clear(key) but I see thatstoreTokenFor does not invokeset.

@fioan89fioan89 merged commit6b00191 intomainSep 13, 2025
6 checks passed
@fioan89fioan89 deleted the fix-dont-store-token-when-auth-via-certificates branchSeptember 13, 2025 10:20
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@code-ashercode-ashercode-asher approved these changes

@matifalimatifaliAwaiting requested review from matifali

@f0sself0sselAwaiting requested review from f0ssel

@jcjiangjcjiangAwaiting requested review from jcjiang

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@fioan89@code-asher

[8]ページ先頭

©2009-2025 Movatter.jp