-
Notifications
You must be signed in to change notification settings - Fork 173
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 cli console project #681
Conversation
In the future, Appwrite won't default to the console project so specifying a project will be required.
A recent change to Appwrite stopped allowing loose matches for endpoints. As such, https://stage.appwrite.io/v1/health/version no longer matches https://stage.appwrite.io/version, causing tests to fail.
@@ -82,6 +82,7 @@ const client = new Command("client") | |||
} | |||
|
|||
let client = new Client().setEndpoint(endpoint); | |||
client.setProject('console'); |
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.
Why do we need this here though?
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.
Currently, Appwrite defaults to the console project if you don't pass a project. This can cause confusing errors so the plan is to not default to the console project. As such, we need to make sure we specify the project here.
tests/languages/cli/test.js
Outdated
@@ -1,6 +1,6 @@ | |||
const { exec, execSync } = require('child_process'); | |||
|
|||
execSync("node index client --endpoint 'https://stage.appwrite.io/v1' --projectId console --key=35y3h5h345 --selfSigned true", { stdio: 'inherit' }); | |||
execSync("node index client --endpoint 'https://demo.appwrite.io/v1' --projectId console --key=35y3h5h345 --selfSigned true", { stdio: 'inherit' }); |
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 need to decide on which instance we want to use for test, also that hast to have latest features even before release. We might want to maintain a separate instance I'm not sure yet but we need a better strategy.
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.
It should be suffice to use any Appwrite instance since this only tests the /v1/health/version
endpoint. Anyways, it's better to get this working than to have it broken.
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.
Looks like this has changed on mater to https://stage.cloud.appwrite.io/v1.
What does this PR do?
Test Plan
Manual
Related PRs and Issues
Have you read the Contributing Guidelines on issues?
Yes