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.

Sign up for GitHub

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

放弃在 save_it bot 添加 notes 功能，保持 save_it 简单 #22

Closed
wants to merge 4 commits into from
Closed

放弃在 save_it bot 添加 notes 功能，保持 save_it 简单 #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
625fdfd
first commit
ThaddeusJiang Oct 25, 2024
256dde8
refactor
ThaddeusJiang Oct 25, 2024
c9bacc8
create & update notes
ThaddeusJiang Oct 25, 2024
5c43f19
fix: miss
ThaddeusJiang Oct 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,14 @@ save_it-*.tar
# Temporary files, for example, from tests.
/tmp/
.DS_Store
data
dev.sh
start.sh
run.sh
nohup.out

# data
_local
data

# scripts
_local*
_dev*
_stag*
_prod*
nohup.out
44 changes: 38 additions & 6 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

A telegram bot can Save photos and Search photos

## Features

- [x] Save photos via a link
- [x] Search photos using semantic search
- [x] Find similar photos by photo
Expand All @@ -27,13 +29,8 @@ messages:

```
/search cat

/search dog

/search girl

/similar photo

/similar photo
```

Expand All @@ -58,10 +55,45 @@ mix deps.get
```

```sh
# run
# start typesense
docker compose up
```

```sh
# modify env
export TELEGRAM_BOT_TOKEN=
export TYPESENSE_URL=
export TYPESENSE_API_KEY=

iex -S mix run --no-halt
```

Pro Tips: create shell script for fast run app

1. touch start.sh

```sh
#!/bin/sh

export TELEGRAM_BOT_TOKEN=<YOUR_TELEGRAM_BOT_TOKEN>

export TYPESENSE_URL=<YOUR_TYPESENSE_URL>
export TYPESENSE_API_KEY=<YOUR_TYPESENSE_API_KEY>

export GOOGLE_OAUTH_CLIENT_ID=<YOUR_GOOGLE_OAUTH_CLIENT_ID>
export GOOGLE_OAUTH_CLIENT_SECRET=<YOUR_GOOGLE_OAUTH_CLIENT_SECRET>

iex -S mix run --no-halt
```

2. execute permission

```sh
chmod +x start.sh
```

3. run

```sh
./start.sh
```
2 changes: 0 additions & 2 deletions config/runtime.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,3 @@ config :save_it, :typesense_api_key, System.get_env("TYPESENSE_API_KEY", "xyz")
# optional
config :save_it, :google_oauth_client_id, System.get_env("GOOGLE_OAUTH_CLIENT_ID")
config :save_it, :google_oauth_client_secret, System.get_env("GOOGLE_OAUTH_CLIENT_SECRET")

config :save_it, :web_url, System.get_env("WEB_URL", "http://localhost:4000")
0 docs/worklogs/2024-10-21.md → docs/dev-logs/2024-10-21.md
View file
Open in desktop
File renamed without changes.
9 changes: 9 additions & 0 deletions docs/dev-logs/2024-10-25.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# 2024-10-25

## Req call typesense API alway :timeout, but typesense was updated.

```elixir
** (MatchError) no match of right hand side value: {:error, %Req.TransportError{reason: :timeout}}
(save_it 0.2.0-rc.1) lib/migration/typesense.ex:11: Migration.Typesense.create_collection!/1
priv/typesense/reset.exs:3: (file)
```
Empty file added docs/dev/readme.md
View file
Open in desktop
Empty file.
14 changes: 14 additions & 0 deletions docs/dev/typesense.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Typesense

## Typesense API Errors

```
# 400 Bad Request - The request could not be understood due to malformed syntax.
# 401 Unauthorized - Your API key is wrong.
# 404 Not Found - The requested resource is not found.
# 409 Conflict - When a resource already exists.
# 422 Unprocessable Entity - Request is well-formed, but cannot be processed.
# 503 Service Unavailable - We’re temporarily offline. Please try again later.
```

docs: https://typesense.org/docs/27.1/api/api-errors.html#api-errors
42 changes: 42 additions & 0 deletions lib/migration/typesense.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
defmodule Migration.Typesense do
def create_collection!(schema) do
req = build_request("/collections")
{:ok, res} = Req.post(req, json: schema)

res.body
end
Comment on lines +2 to +7
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add proper error handling and documentation for public functions.

The public API functions have several areas for improvement:

  1. Error handling: The functions assume requests will succeed ({:ok, res}), but network issues or API errors could occur.
  2. Input validation: Parameters should be validated before making requests.
  3. Documentation: Public functions should have @doc and @spec annotations.
  4. Response consistency: Consider wrapping response bodies in a consistent structure.

Here's a suggested improvement:

+ @doc """
+ Creates a new collection in Typesense.
+ 
+ ## Parameters
+   - schema: Map containing the collection schema
+ 
+ ## Returns
+   - {:ok, response} on success
+   - {:error, reason} on failure
+ """
+ @spec create_collection!(map()) :: {:ok, map()} | {:error, any()}
 def create_collection!(schema) do
+  with {:ok, schema} <- validate_schema(schema),
+       req <- build_request("/collections"),
+       {:ok, res} <- Req.post(req, json: schema) do
+    {:ok, res.body}
+  else
+    {:error, reason} -> {:error, reason}
+    error -> {:error, error}
+  end
 end

+ @doc """
+ Deletes a collection from Typesense.
+ 
+ ## Parameters
+   - collection_name: String name of the collection
+ 
+ ## Returns
+   - {:ok, response} on success
+   - {:error, reason} on failure
+ """
+ @spec delete_collection!(String.t()) :: {:ok, map()} | {:error, any()}
 def delete_collection!(collection_name) do
+  with {:ok, name} <- validate_collection_name(collection_name),
+       req <- build_request("/collections/#{name}"),
+       {:ok, res} <- Req.delete(req) do
+    {:ok, res.body}
+  else
+    {:error, reason} -> {:error, reason}
+    error -> {:error, error}
+  end
 end

Add these private validation functions:

defp validate_schema(%{} = schema) do
  {:ok, schema}
end
defp validate_schema(_), do: {:error, "Invalid schema format"}

defp validate_collection_name(name) when is_binary(name) and byte_size(name) > 0 do
  {:ok, name}
end
defp validate_collection_name(_), do: {:error, "Invalid collection name"}

Also applies to: 9-14, 16-21


def delete_collection!(collection_name) do
req = build_request("/collections/#{collection_name}")
{:ok, res} = Req.delete(req)

res.body
end

def list_collections() do
req = build_request("/collections")
{:ok, res} = Req.get(req)

res.body
end

defp get_env() do
url = Application.fetch_env!(:save_it, :typesense_url)
api_key = Application.fetch_env!(:save_it, :typesense_api_key)

{url, api_key}
end
Comment on lines +23 to +28
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider safer environment variable handling.

The get_env/0 function uses fetch_env! which raises an error if the configuration is missing. This could cause runtime crashes in production.

Consider using Application.get_env/3 with fallbacks or environment-specific validation:

 defp get_env() do
-  url = Application.fetch_env!(:save_it, :typesense_url)
-  api_key = Application.fetch_env!(:save_it, :typesense_api_key)
-
-  {url, api_key}
+  with {:ok, url} <- get_config(:typesense_url),
+       {:ok, api_key} <- get_config(:typesense_api_key) do
+    {:ok, {url, api_key}}
+  end
 end

+ defp get_config(key) do
+  case Application.get_env(:save_it, key) do
+    nil -> {:error, "Missing configuration: #{key}"}
+    value when byte_size(value) > 0 -> {:ok, value}
+    _ -> {:error, "Invalid configuration: #{key}"}
+  end
+ end

Committable suggestion was skipped due to low confidence.


defp build_request(path) do
{url, api_key} = get_env()

Req.new(
base_url: url,
url: path,
headers: [
{"Content-Type", "application/json"},
{"X-TYPESENSE-API-KEY", api_key}
]
)
end
Comment on lines +30 to +41
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add request timeout configuration and consider making headers configurable.

The request configuration could be improved with timeout settings and more flexible header configuration.

Consider these improvements:

 defp build_request(path) do
-  {url, api_key} = get_env()
+  with {:ok, {url, api_key}} <- get_env() do
+    Req.new(
+      base_url: url,
+      url: path,
+      headers: build_headers(api_key),
+      connect_options: [timeout: get_timeout()],
+      retry: :transient
+    )
+  end
 end

+ defp build_headers(api_key) do
+  [
+    {"Content-Type", "application/json"},
+    {"X-TYPESENSE-API-KEY", api_key}
+  ]
+ end

+ defp get_timeout do
+  Application.get_env(:save_it, :typesense_timeout, 5000)
+ end
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
defp build_request(path) do
{url, api_key} = get_env()
Req.new(
base_url: url,
url: path,
headers: [
{"Content-Type", "application/json"},
{"X-TYPESENSE-API-KEY", api_key}
]
)
end
defp build_request(path) do
with {:ok, {url, api_key}} <- get_env() do
Req.new(
base_url: url,
url: path,
headers: build_headers(api_key),
connect_options: [timeout: get_timeout()],
retry: :transient
)
end
end
defp build_headers(api_key) do
[
{"Content-Type", "application/json"},
{"X-TYPESENSE-API-KEY", api_key}
]
end
defp get_timeout do
Application.get_env(:save_it, :typesense_timeout, 5000)
end

end
30 changes: 30 additions & 0 deletions lib/migration/typesense/note.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
defmodule Migration.Typesense.Note do
alias Migration.Typesense

@notes_schema %{
"name" => "notes",
"fields" => [
# TODO: 第一步先实现文本，今后再考虑图片
%{"name" => "content", "type" => "string"},
# references photos.id
# note: 抉择：这个 app 核心是给予图片的视觉笔记，暂时不考虑单独 text 的笔记
# %{"name" => "photo_id", "type" => "string"},
# note: 既然不能实现 RDB reference，那么就直接存储 file_id
%{"name" => "message_id", "type" => "string"},
%{"name" => "file_id", "type" => "string"},
%{"name" => "belongs_to_id", "type" => "string"},
%{"name" => "inserted_at", "type" => "int64"},
%{"name" => "updated_at", "type" => "int64"}
],
"default_sorting_field" => "inserted_at"
}

def create_collection!() do
Typesense.create_collection!(@notes_schema)
end

def reset!() do
Typesense.delete_collection!(@notes_schema["name"])
Typesense.create_collection!(@notes_schema)
end
end
35 changes: 35 additions & 0 deletions lib/migration/typesense/photo.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
defmodule Migration.Typesense.Photo do
alias Migration.Typesense

@photos_schema %{
"name" => "photos",
"fields" => [
# image: base64 encoded string
%{"name" => "image", "type" => "image", "store" => false},
%{
"name" => "image_embedding",
"type" => "float[]",
"embed" => %{
"from" => ["image"],
"model_config" => %{
"model_name" => "ts/clip-vit-b-p32"
}
}
},
%{"name" => "caption", "type" => "string", "optional" => true, "facet" => false},
%{"name" => "file_id", "type" => "string"},
%{"name" => "belongs_to_id", "type" => "string"},
%{"name" => "inserted_at", "type" => "int64"}
],
"default_sorting_field" => "inserted_at"
}

def create_collection!() do
Typesense.create_collection!(@photos_schema)
end

def reset!() do
Typesense.delete_collection!(@photos_schema["name"])
Typesense.create_collection!(@photos_schema)
end
Comment on lines +27 to +34
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add documentation and error handling.

The public functions need:

  1. @doc and @spec annotations
  2. Error handling for Typesense operations
  3. Return value documentation

Consider this improvement:

+  @doc """
+  Creates a new photos collection in Typesense.
+  
+  Returns `:ok` on success, or raises an error if the collection already exists
+  or if there's a connection issue.
+  """
+  @spec create_collection!() :: :ok | no_return()
   def create_collection!() do
-    Typesense.create_collection!(@photos_schema)
+    case Typesense.create_collection!(@photos_schema) do
+      {:ok, _} -> :ok
+      {:error, %{"message" => message}} -> raise "Failed to create collection: #{message}"
+    end
   end

+  @doc """
+  Resets the photos collection by deleting and recreating it.
+  
+  Returns `:ok` on success, or raises an error if the operations fail.
+  """
+  @spec reset!() :: :ok | no_return()
   def reset!() do
-    Typesense.delete_collection!(@photos_schema["name"])
-    Typesense.create_collection!(@photos_schema)
+    with :ok <- Typesense.delete_collection!(@photos_schema["name"]),
+         :ok <- create_collection!() do
+      :ok
+    end
   end
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def create_collection!() do
Typesense.create_collection!(@photos_schema)
end
def reset!() do
Typesense.delete_collection!(@photos_schema["name"])
Typesense.create_collection!(@photos_schema)
end
@doc """
Creates a new photos collection in Typesense.
Returns `:ok` on success, or raises an error if the collection already exists
or if there's a connection issue.
"""
@spec create_collection!() :: :ok | no_return()
def create_collection!() do
case Typesense.create_collection!(@photos_schema) do
{:ok, _} -> :ok
{:error, %{"message" => message}} -> raise "Failed to create collection: #{message}"
end
end
@doc """
Resets the photos collection by deleting and recreating it.
Returns `:ok` on success, or raises an error if the operations fail.
"""
@spec reset!() :: :ok | no_return()
def reset!() do
with :ok <- Typesense.delete_collection!(@photos_schema["name"]),
:ok <- create_collection!() do
:ok
end
end

end
Loading
Loading