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(cli-2.0): initial draft #28

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

feat(cli-2.0): initial draft #28

wants to merge 11 commits into from

Conversation

christyjacob4
Copy link
Member

@christyjacob4 christyjacob4 commented Jul 14, 2021

018-cli-2.0/018-cli-2.0.md Outdated Show resolved Hide resolved
018-cli-2.0/018-cli-2.0.md Outdated Show resolved Hide resolved
018-cli-2.0/018-cli-2.0.md Show resolved Hide resolved
018-cli-2.0/018-cli-2.0.md Outdated Show resolved Hide resolved
018-cli-2.0/018-cli-2.0.md Outdated Show resolved Hide resolved
018-cli-2.0/018-cli-2.0.md Outdated Show resolved Hide resolved
018-cli-2.0/018-cli-2.0.md Outdated Show resolved Hide resolved
Co-authored-by: Eldad A. Fux <eldad.fux@gmail.com>
Copy link
Member

@lohanidamodar lohanidamodar left a comment

Choose a reason for hiding this comment

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

Some suggestions

temp
├── appwrite.json
└── FunctionOne
├── .appwrite
Copy link
Member

Choose a reason for hiding this comment

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

May be some information regarding .appwrite file will be useful as well

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to have everything in appwrite.json? I think it's a bit confusing to have two different types of config files.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe most major configs are done using appwrite.json then .appwrite can be used for API keys and project keys as it'll be hidden by default right? Aslong as we remind users not to commit .appwrite they can share their project with ease

Copy link

Choose a reason for hiding this comment

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

Agreeing with @kodumbeats - If we are going to have a .appwrite directory, why not have config.yaml in there (example, how .git is setup) or just have appwrite.yaml

Copy link
Member Author

Choose a reason for hiding this comment

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

@lohanidamodar .appwrite is actually the folder which some runtimes use to store their dependencies like Python and ruby . Similar to node modules in npm projects

Copy link
Member Author

Choose a reason for hiding this comment

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

@kodumbeats appwrite.json is a file that is expected to be a part of version control since it will have all the information related to a project like the collections, functions and so on and hence allow them to recreate their project environment . Having secrets in this file doesn't seem like a good idea



# Usage 2
appwrite generate function --name="FunctionTwo" --runtime=node-16.0
Copy link
Member

Choose a reason for hiding this comment

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

May be we can introduce one more parameter here --template which should be a link to an accessible github repository/branch so that devs can customize to use their own templates as well as our more advance templates, even our demos can be used as template easily using this feature?

"path" : "./FunctionOne",
"command" : "python main.py",
"runtime": "python3.9",
// This section is useful when we allow cloud functions to be tested locally
Copy link
Member

Choose a reason for hiding this comment

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

JSON can't have comments 😉

# Usage 2
appwrite deploy functions --all
# Deploys all the functions specified in appwrite.json

Choose a reason for hiding this comment

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

What do you think about adding a 4th usage for CI environments to deploy some of the projects functions:

appwrite deploy functions --names "My awesome function 💪, FunctionTwo" or
appwrite deploy functions --names "My awesome function 💪" "FunctionTwo"

Copy link

Choose a reason for hiding this comment

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

With this and others, I believe we should name space the commands functions deploy (and looking further down, site deploy -y


```sh
# Usage 1
appwrite deploy functions --confirm

Choose a reason for hiding this comment

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

For simplicity of use, one of --confirm or --all could be a default

Copy link
Contributor

Choose a reason for hiding this comment

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

This almost feels like an -i --interactivemode, like an interactive rebase in git?

Copy link

Choose a reason for hiding this comment

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

could also follow the common CLI trends with -y instead of a --confirm or --all

Here are some example usages
```sh
# Usage 1
appwrite init function --functionId="abc" --name="FunctionOne" --runtime=python-3.9

Choose a reason for hiding this comment

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

Maybe we could allow runtime to be a language and use the latest version available by default? e.g.:

appwrite init function --functionId="abc" --name="FunctionOne" --runtime=python

Which would use python-3.9 or whichever is newest at the time

Copy link

Choose a reason for hiding this comment

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

Like above: appwrite function init -- This helps when printing help screens too, because you can. order them by the namespaces for readable grouping


After the implementation of this RFC, the CLI will behave more like the Appwrite Console rather than a Server SDK. This accounts for a new form of Authentication that is similar to the way we handle logins in the Appwrite console. We use a cookie.

We will introduce a new command `appwrite login` that will work in one of either ways.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to pick one or the other? I understand that we should think to support OAuth login, but it's not a great CLI experience if a browser is required to login.

# Usage 3
appwrite init function

Give your function an ID
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to validate this ID with the Key validator as a part of the CLI? We could provide real-time feedback even before a request is made.

-->

This RFC is inspired by the Firebase CLI.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does the Fission CLI have any insights to offer?
https://docs.fission.io/docs/usage/function/functions/

@Meldiron
Copy link

Meldiron commented Aug 4, 2021

Local testing

Defining testing environments inside appwrte.json looks promising, but will we be able to write multiple tests? I believe all scripts are supposed to return different outputs for different inputs, so we should have an option to write multiple tests. For example

  • Register newsletter should return "success" if the email is "matejbaco2000@gmail.com"
  • It should return "error" if the email is "h93grnfb9qewn"
  • It should return "error" if the Recaptcha result is incorrect

I can see a solution where instead of "vars" we have "tests" objects like this:

tests: [
  {
      vars: { email: "matejbaco2000@gmail.com" },
      name: "it should return success with correct email",
      expectedLog: "success"
    },
  {
      vars: { email: "blabla" },
      name: "it should return error with wrong email",
      expectedLog: "error"
    }
]

This is a pretty simple example because we might be looking for more statements such as isNull or isGreaterThan. the expectedLog could follow syntax similar to new database refactoring filters so it could look something like:

tests: [
  {
      vars: { a: 5, b: 8, method: "+" },
      name: "it should equal  5 + 8 = 13",
      expectedLog: "this.result.equalTo(13)" // for output: { result: 13 }
    },
  {
      vars: { a: 5, b: 8, method: "-" },
      name: "it should equal  5 - 8 = -3",
      expectedLog: "this.result.equalTo(-3)" // for output: { result: -3 }
    },
  {
      vars: { a: 2423423, b: 1231412323, method: "*" },
      name: "it should equal  be huge number",
      expectedLog: "this.result.higherThan(999999999)"
    },
   {
      vars: { a: 5, b: 0, method: "/" },
      name: "it should throw error  5 / 0",
      expectedLog: "this.errorMsg.equalTo('Wrong input')" // for output: { result: null, errorMsg: "Wrong input" }
    },
]

.appwriteignore

If we build the application, we only want to deploy the build folder. That would be possible.
Problem is, what if we don't build our code and we need to deploy the whole folder? This folder can include .git, .env and other files that may include secret information that we don't want to ship with a build.

Also, if we have a build process in place, I may have node_modules filled with a bunch of development libraries that are not required for production. Can I somehow remove them from deploy to keep it smaller?

Database setup

Since we can set up sites and function, can we also add an option to define collection structure using appwrite CLI? There could be database folder that would hold migrations and seeds, similar to how MySQL does it (Knex for example). Migrations define the scheme of a collection and seeds fill the collection with sample documents. This would also get really related to custom IDs in future, since I probably want to use custom IDs for seed documents (such as Settings document)

@Meldiron
Copy link

Meldiron commented Aug 8, 2021

Function templates

Current RFC includes command:

appwrite init function --functionId="abc" --name="FunctionOne" --runtime=python-3.9

For python, it generates files main.py and requirements.txt. For Node, I would expect package.jsonand app.js.

What if we want to use Typescript? In the end, we are still going to use Node (or Deno) runtime, but the function structure needs to be way different. First of all, we need tsconfig.json and code In src folder. Then, we need a build function. Finally, we need to deploy a specific folder (+ node_modules?).

In my opinion, the init function could get one more parameter:

appwrite init function --functionId="abc" --name="FunctionOne" --runtime=python-3.9 --template=python

There will be multiple templates defined by Appwrite for the most common use cases. This parameter could also support GIT reference, so people can create their own templates. I don't think there is much that the template needs to know. Information such as relative path, function name or runtime can be easily passed using handlebars.

@stnguyen90
Copy link

The current CLI is pretty slow because it has to spin up a docker container to execute one command and then tear down.

In addition, passing objects into the CLI is incredibly difficult because of how the data is passed into the container and parsed via the CLI code (Discord conversation)

Any plans on rewriting the CLI to not be a docker image like this?

},
],
"hosting": []
}
Copy link

Choose a reason for hiding this comment

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

I would like to suggest YAML or even TOML, support for them everywhere, easier to read and less prone to type-o frustration with quotes and such. Also... comments.


```sh
appwrite init

Copy link

@wess wess Nov 5, 2021

Choose a reason for hiding this comment

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

its this to add the Appwrite stack, to setup for functions or just create the config file with the project id? If project, will it also create the project for us on our endpoints?

@stnguyen90
Copy link

@christyjacob4, what are the next steps on this?

@stnguyen90 stnguyen90 marked this pull request as draft October 5, 2023 17:27
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.

9 participants