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

Update README with workaround for error caused by length requirement on token property even if not required by server #63

Merged
merged 9 commits into from
Oct 18, 2024

Conversation

knutle
Copy link
Contributor

@knutle knutle commented Oct 13, 2024

Add a quick note to explain a workaround for incorrect token error even when using local provider with no auth required.

Root cause seems to be the string length check on token at plugins/openai_worker.py:471
Should be a fairly quick fix depending on how you prefer to handle it, but I'd probably suggest something like disabling the length check if token value is explicitly set to Null rather that empty string just for simplicity.

Anyways, I figured a little heads up added to the readme would have saved me a bit of head-scratching, while still being fairly low-effort in case you already have plans for a long term fix, so here it is :)

@yaroslavyaroslav
Copy link
Owner

Hey, thank you for participating with this. It's really quite painfully missed info within the docs. But could you also please add kinda same note into openAI.sublime-settings right above the "token" field, to make it even more eye catchy?

@knutle
Copy link
Contributor Author

knutle commented Oct 13, 2024

Sure, no problem! How about adding a new "Known Issues" section right before "Disclaimers"? Explain how to implement the workaround there, and link it up to an actual repo issue which actually details the underlying cause.

Then just briefly mention the issue where relevant and reference the full explanation. Allows the fix to be more prominent without adding too much extra noise.

I'll tinker with it a little and add some suggestions :)

…en when using local provider with no auth required.

Squashed commit of the following:

commit 92d5e6a
Author: Knut Leborg <[email protected]>
Date:   Sun Oct 13 13:14:50 2024 +0200

    Update README.md

commit 750dcae
Author: Knut Leborg <[email protected]>
Date:   Sun Oct 13 13:11:00 2024 +0200

    Update README.md

commit fe7650c
Author: Knut Leborg <[email protected]>
Date:   Sun Oct 13 13:07:52 2024 +0200

    Update README.md

commit 0e16178
Author: Knut Leborg <[email protected]>
Date:   Sun Oct 13 13:01:34 2024 +0200

    Update README.md

    Add a quick note to explain a workaround for incorrect token error even when using local provider with no auth required.
@knutle knutle marked this pull request as draft October 14, 2024 01:12
@knutle
Copy link
Contributor Author

knutle commented Oct 14, 2024

See issue #64 for a detailed description of problem cause, current workaround, and other relevant info

@knutle knutle marked this pull request as ready for review October 14, 2024 07:30
@knutle
Copy link
Contributor Author

knutle commented Oct 14, 2024

let me know if i missed something ✌️
also, if my suggested solution sounds good I can probably take a stab at it around the end of the week if you'd like👌

@yaroslavyaroslav
Copy link
Owner

Thank you for such thorough PR.

The only thing that catches my eye here is that I consider as no go to ask a user to go to GitHub issues to get the answer of how to fix the error they're got, so I'd add this very comment in the settings after all // Token can be anything so long as it is at least 10 characters long.

But if you're gonna to fix the issue completely soon, I'm ok with how it's going as well.

@knutle
Copy link
Contributor Author

knutle commented Oct 18, 2024

Updated the comment in the config file as suggested since I might not have time to fix the issue for a little while after all, sorry 😅

@yaroslavyaroslav
Copy link
Owner

No worries, there is no rush here. Thank you for the participation with docs.

@yaroslavyaroslav yaroslavyaroslav merged commit 2eb6792 into yaroslavyaroslav:develop Oct 18, 2024
@knutle knutle deleted the patch-1 branch October 20, 2024 14:42
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.

2 participants