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

feat: appwrite adapter #10000

Open
wants to merge 55 commits into
base: main
Choose a base branch
from

Conversation

sourabpramanik
Copy link

@sourabpramanik sourabpramanik commented Feb 11, 2024

Covered most of the cases and methods. Follow the to-do to know about the upcoming changes:

  • Docker setup for testing
  • Shell script to automate initialization
  • Fix Verification token methods
  • Add detailed steps for using the adapter, and process to set up the database, collections, and attributes.
  • Clean up

☕️ Reasoning

This PR is for adding the Appwrite adapter for auth.js and next-auth.js

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

🎫 Affected issues

Feat #7709

📌 Resources

Copy link

vercel bot commented Feb 11, 2024

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

Name Status Preview Comments Updated (UTC)
auth-docs ❌ Failed (Inspect) Jun 27, 2024 1:45pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
next-auth-docs ⬜️ Ignored (Inspect) Visit Preview Jun 27, 2024 1:45pm

Copy link

vercel bot commented Feb 11, 2024

@sourabpramanik is attempting to deploy a commit to the authjs Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions bot added the adapters Changes related to the core code concerning database adapters label Feb 11, 2024
@ndom91
Copy link
Member

ndom91 commented Feb 14, 2024

Hey @sourabpramanik thanks for the contribution! Without having taken a closer look at the code, it seems like you've got a lot of the basics covered. As you mentioned in your todo list above, a testing setup is required.

I'd encourage you to first take a look at our upcoming "creating a database adapter guidelines" and double check you've got everythign covered 🙏

Thanks!

@sourabpramanik sourabpramanik changed the title feat: added appwrite adapter (WIP) feat: appwrite adapter (WIP) Feb 14, 2024
@sourabpramanik
Copy link
Author

sourabpramanik commented Feb 14, 2024

Hey @sourabpramanik thanks for the contribution! Without having taken a closer look at the code, it seems like you've got a lot of the basics covered. As you mentioned in your todo list above, a testing setup is required.

I'd encourage you to first take a look at our upcoming "creating a database adapter guidelines" and double check you've got everythign covered 🙏

Thanks!

Yes, only thing that is left is to add the documentation to explain the structure of the database and the test script. The test works fine when you have the following:

  • Appwrite cloud account
  • Existing project in appwrite
    So a fully automated script that can create a project is not possible without having an account. What should I do?

I have tested this locally all the test cases were passing and created a NextJS project with NextAuth to use this Adapter with the GitHub provider and Email provider, there too this adapter works well. The logic is working fine just stuck at writing the test script for it to automate.

@ndom91
Copy link
Member

ndom91 commented Feb 14, 2024

Is there a local dev package for the appwrite maybe? Like the "firebase local emulator" in the case if firestore 🤔

Having a set of tests for which we need a cloud account is not something we can accept unfortunately

@sourabpramanik
Copy link
Author

No there's no other option I could find. But I am talking to the Appwrite team about this. Let's see.

@sourabpramanik
Copy link
Author

sourabpramanik commented Feb 15, 2024

@ndom91 the Upstash Redis adapter has a test dependency on Redis URL and Key similar so if that can work then the Appwrite adapter can also work right. For appwrite we will need client Id, endpoint, and API secret.

@ndom91
Copy link
Member

ndom91 commented Feb 15, 2024

@ndom91 the Upstash Redis adapter has a test dependency on Redis URL and Key similar so if that can work then the Appwrite adapter can also work right. For appwrite we will need client Id, endpoint, and API secret.

Hmm odd, i don't remember ever setting an upstash api key anywhere locally. They even reference a local-only alternative for testing / development in their docs (https://upstash.com/docs/oss/sdks/ts/redis/developing).

Let us know if you find a solution 🙏 I'll talk to the other folks on the team about what they think in the mean time

@sourabpramanik
Copy link
Author

Sure I am working on it. Thankyou

@ndom91
Copy link
Member

ndom91 commented Feb 15, 2024

So we really want to avoid another cloud provider requirement for tests. The upstash one has been nothing but a pain in the butt, in fact i just refactored it out after i found the serverless-redis-http recommendation from them while researching this haha. See #10033

Anyway, we're totally happy to accept new adapters like this, but please no cloud requirements for testing 🙏

@sourabpramanik
Copy link
Author

sourabpramanik commented Feb 15, 2024

Yes it does make sense to automate such an extra process so that we can only focus on the parts we care about.

I will raise a request about it in today's office hours and let's see if there's a way we can avoid using the cloud. If not then all the hard work will be for nothing at all 🥲

@DH-555
Copy link

DH-555 commented Feb 15, 2024

So we really want to avoid another cloud provider requirement for tests. The upstash one has been nothing but a pain in the butt, in fact i just refactored it out after i found the serverless-redis-http recommendation from them while researching this haha. See #10033

Anyway, we're totally happy to accept new adapters like this, but please no cloud requirements for testing 🙏

@ndom91 Appwrite contributor here.
Appwrite is open source so it can be Self-hosted very easily in your own machine or in your own small VPS with a single command after installing docker:
https://appwrite.io/docs/advanced/self-hosting

Is that enough for this case?

Thanks!

@sourabpramanik
Copy link
Author

Hey @PineappleIOnic I am unable to create an account the response is not 201. Am I missing anything?

@PineappleIOnic
Copy link

@sourabpramanik What command are you running? You shouldn't need to source anything really just run pnpm exec turbo test --concurrency 1 "--filter=*appwrite*" also make sure you don't have any volumes left over from a previous run

@sourabpramanik
Copy link
Author

@sourabpramanik What command are you running? You shouldn't need to source anything really just run pnpm exec turbo test --concurrency 1 "--filter=*appwrite*" also make sure you don't have any volumes left over from a previous run

Yes using this command. Maybe try removing the volumes

@sourabpramanik
Copy link
Author

Hey @PineappleIOnic I am not having any luck. What should I do?
image

@PineappleIOnic
Copy link

@sourabpramanik Your in our discord server right? We can hop on a call if you like and we can debug it together on your machine. Shoot me a DM and I'll try and organize a time

@sourabpramanik
Copy link
Author

@sourabpramanik Your in our discord server right? We can hop on a call if you like and we can debug it together on your machine. Shoot me a DM and I'll try and organize a time

Thanks man but I fixed it by increasing the setTimeout duration from 1 sec to 5 sec. The database setup was still in progress while the server responded with 200 status.

@sourabpramanik
Copy link
Author

database functions are too slow and cause the tests to fail. Did you experience the lag?

@PineappleIOnic
Copy link

I've never really experienced it being slow on my end however if need be we could implement checks to make sure the schema has been correctly setup before tests run, something like "check if DB is full provisioned, if not wait 5 seconds and try again" and we loop for like 5 tries, wdyt?

@sourabpramanik
Copy link
Author

I've never really experienced it being slow on my end however if need be we could implement checks to make sure the schema has been correctly setup before tests run, something like "check if DB is full provisioned, if not wait 5 seconds and try again" and we loop for like 5 tries, wdyt?

Worth a try. I will get back with the results

@sourabpramanik sourabpramanik changed the title feat: appwrite adapter (WIP) feat: appwrite adapter Jun 6, 2024
@sourabpramanik
Copy link
Author

@ndom91 Can you please review this again? @PineappleIOnic has improved the test setup process and updated the docs, so now everything should work as expected for you.

Looking forward!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adapters Changes related to the core code concerning database adapters
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants