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

Zep Authentication #6728

Merged
merged 7 commits into from
Jun 30, 2023
Merged

Zep Authentication #6728

merged 7 commits into from
Jun 30, 2023

Conversation

danielchalef
Copy link
Contributor

@danielchalef danielchalef commented Jun 25, 2023

Description: Add Zep API Key argument to ZepChatMessageHistory and ZepRetriever

  • correct docs site links
  • add zep api_key auth to constructors

ZepChatMessageHistory: @hwchase17,
ZepRetriever: @rlancemartin, @eyurtsev

@vercel
Copy link

vercel bot commented Jun 25, 2023

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

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Jun 30, 2023 9:17pm

@danielchalef danielchalef reopened this Jun 25, 2023
@rlancemartin rlancemartin self-assigned this Jun 26, 2023
@rlancemartin
Copy link
Collaborator

Thanks! Maybe you can also re-run the notebook to confirm all is working following the update.

@danielchalef
Copy link
Contributor Author

Thanks! Maybe you can also [re-run the notebook]

@rlancemartin Good idea. Done!

@danielchalef danielchalef marked this pull request as ready for review June 26, 2023 23:42
@danielchalef
Copy link
Contributor Author

@rlancemartin bump on this. Would love to get this merged as we have users interested in using authentication. Thanks!

@rlancemartin
Copy link
Collaborator

@danielchalef looks good! Can you rebase or merge master, resolve conflicts in pyproject.toml and then run poetry lock --no-update to re-generate poetry.lock? I can help if needed.

@danielchalef
Copy link
Contributor Author

@rlancemartin Done. Waiting on CI tests to pass.

@rlancemartin
Copy link
Collaborator

Thanks! Hmm, still looks like a poetry.lock conflict?

@danielchalef
Copy link
Contributor Author

Thanks! Hmm, still looks like a poetry.lock conflict?

Weird. Looks like this happened over night. There wasn't a conflict yesterday. I've tried rebasing again but am up to date. Will delete and recreate the file.

@rlancemartin
Copy link
Collaborator

Thanks! Hmm, still looks like a poetry.lock conflict?

Weird. Looks like this happened over night. There wasn't a conflict yesterday. I've tried rebasing again but am up to date. Will delete and recreate the file.

Thanks, ya sorry about that. LMK if issues. I can also try on my end.

Usually just:

checkout master poetry.lock
poetry lock --no-update

Should do it.

@danielchalef
Copy link
Contributor Author

@rlancemartin done. no merge issues. tests are currently running.

@danielchalef
Copy link
Contributor Author

Ugh. Fixing this. Two minutes

@rlancemartin
Copy link
Collaborator

Ha, ya. A lot of trouble w/ the pyproject.toml :P ... looks like all other tests are passing.

@danielchalef
Copy link
Contributor Author

danielchalef commented Jun 30, 2023

Yes. I keep running poetry lock --no-update and it doesn't produce a different output for the pytproject file I've checked in. The poetry.lock file has a different hash to what is in master. And that's the Only difference.

Would you mind taking it from here?

@rlancemartin
Copy link
Collaborator

rlancemartin commented Jun 30, 2023

Yes. I keep running poetry lock --no-update and it doesn't produce a different output for the pytproject file I've checked in. The poetry.lock file has a different hash to what is in master. And that's the Only difference.

Would you mind taking it from here?

Looks like I can't push to your branch. But I checked out your branch and did this:

1/ sync for your fork w/ master
2/ on master, update with git pull
3/ on your branch (zep-jwt-auth): git checkout master poetry.lock
4/ then poetry lock --no-update

This should resolve it.

Maybe your master is a bit out of date?

@rlancemartin
Copy link
Collaborator

1/ sync for your fork w/ master 2/ on master, update with git pull 3/ on your branch (zep-jwt-auth): git checkout master poetry.lock 4/ then poetry lock --no-update

I see the same conflict on another PR after doing the above.

Just pinged @dev2049 on it.

@rlancemartin
Copy link
Collaborator

rlancemartin commented Jun 30, 2023

1/ sync for your fork w/ master 2/ on master, update with git pull 3/ on your branch (zep-jwt-auth): git checkout master poetry.lock 4/ then poetry lock --no-update

I see the same conflict on another PR after doing the above.

Just pinged @dev2049 on it.

It was resolved by ensuring local master is up-to-date, as mentioned above.

Can you confirm that and then do the steps above?

In parallel I am getting #6772 ready to merge (to ensure no other issues).

I'd prefer to get this in first.

@danielchalef
Copy link
Contributor Author

Doing so now. I'm a little confused why rebasing against upstream/master wasn't resolving this.

@rlancemartin
Copy link
Collaborator

Doing so now. I'm a little confused why rebasing against upstream/master wasn't resolving this.

It should. #6772 is now fixed, too.

I think it's possible that your local master was out of date.

Looks like the conflict is resolved!

Let's get this in once tests pass.

Sorry for the trouble.

@danielchalef
Copy link
Contributor Author

No worries. I think that was the case. Thanks

@rlancemartin rlancemartin merged commit b26cca8 into langchain-ai:master Jun 30, 2023
14 checks passed
rlancemartin added a commit to hemidactylus/langchain that referenced this pull request Jun 30, 2023
rlancemartin added a commit to hemidactylus/langchain that referenced this pull request Jun 30, 2023
vowelparrot pushed a commit that referenced this pull request Jul 4, 2023
## Description: Add Zep API Key argument to ZepChatMessageHistory and
ZepRetriever
- correct docs site links
- add zep api_key auth to constructors

ZepChatMessageHistory: @hwchase17, 
ZepRetriever: @rlancemartin, @eyurtsev
@danielchalef danielchalef deleted the zep-jwt-auth branch July 4, 2023 17:09
aerrober pushed a commit to aerrober/langchain-fork that referenced this pull request Jul 24, 2023
## Description: Add Zep API Key argument to ZepChatMessageHistory and
ZepRetriever
- correct docs site links
- add zep api_key auth to constructors

ZepChatMessageHistory: @hwchase17, 
ZepRetriever: @rlancemartin, @eyurtsev
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.

None yet

2 participants