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: switch from mangodb to postgres #30

Closed
wants to merge 1 commit into from
Closed

Conversation

Mohammad96Assaf
Copy link
Contributor

@Mohammad96Assaf Mohammad96Assaf commented Aug 3, 2024

swithching from mangodb to postgreSQL

Close #6

Copy link
Member

@choffmann choffmann left a comment

Choose a reason for hiding this comment

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

Please implement the feedback. I can't test the application because it won't build and there are some issues I mentioned in the review. If you have fixed them, I would like to do another review.

@@ -10,7 +10,7 @@ const (

type SensorPredictionResponse struct {
SensorID string `json:"sensor_id"`
Tree *TreeResponse `json:"tree"`
Tree *TreeResponse `json:"treeSQL"`
Copy link
Member

Choose a reason for hiding this comment

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

Why treeSQL? Please let it by tree

Suggested change
Tree *TreeResponse `json:"treeSQL"`
Tree *TreeResponse `json:"tree"`

@@ -3,12 +3,12 @@ package tree
import "github.com/green-ecolution/green-ecolution-backend/internal/server/http/entities/sensor"

type TreeSensorPredictionResponse struct {
Tree *TreeResponse `json:"tree,omitempty"`
Tree *TreeResponse `json:"treeSQL,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Tree *TreeResponse `json:"treeSQL,omitempty"`
Tree *TreeResponse `json:"tree,omitempty"`

SensorPrediction *SensorPredictionResponse `json:"sensor_prediction,omitempty"`
SensorData []*sensor.MqttPayloadResponse `json:"sensor_data,omitempty" extensions:"x-nullable"`
} // @Name TreeSensorPrediction

type TreeSensorDataResponse struct {
Tree *TreeResponse `json:"tree,omitempty"`
Tree *TreeResponse `json:"treeSQL,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Tree *TreeResponse `json:"treeSQL,omitempty"`
Tree *TreeResponse `json:"tree,omitempty"`

SensorPrediction *SensorPredictionResponse `json:"sensor_prediction,omitempty"`
SensorData []*sensor.MqttPayloadResponse `json:"sensor_data,omitempty" extensions:"x-nullable"`
} // @Name TreeSensorPrediction

type TreeSensorDataResponse struct {
Tree *TreeResponse `json:"tree,omitempty"`
Tree *TreeResponse `json:"treeSQL,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Tree *TreeResponse `json:"treeSQL,omitempty"`
Tree *TreeResponse `json:"tree,omitempty"`

Comment on lines +16 to +17
// @Param sensor_data query boolean false "Get raw sensorSQL data for each treeSQL"
// @Success 200 {object} []treeSQL.TreeSensorDataResponse
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// @Param sensor_data query boolean false "Get raw sensorSQL data for each treeSQL"
// @Success 200 {object} []treeSQL.TreeSensorDataResponse
// @Param sensor_data query boolean false "Get raw sensor data for each tree"
// @Success 200 {object} []tree.TreeSensorDataResponse

return data, nil
}

func (r *TreeRepository) GetAll(ctx context.Context) ([]*tree.TreeEntity, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Return the domain entity

Suggested change
func (r *TreeRepository) GetAll(ctx context.Context) ([]*tree.TreeEntity, error) {
func (r *TreeRepository) GetAll(ctx context.Context) ([]*tree.Tree, error) {

Copy link
Member

Choose a reason for hiding this comment

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

Inside the mapper, remove the mongodb stuff

func (r *SchemaRepository) SetupSensorTable() {
_, err := r.db.Exec(`
CREATE TABLE IF NOT EXISTS sensors (
id UUID PRIMARY KEY,
Copy link
Member

Choose a reason for hiding this comment

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

We should not use uuid's in a sql database as the primary key

type Repository struct {
Info InfoRepository
Sensor SensorRepository
Tree TreeRepository
Schema SchemaRepository
Copy link
Member

Choose a reason for hiding this comment

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

We dont need this

@@ -75,6 +75,7 @@ func main() {
Info: localRepo.Info,
Sensor: dbRepo.Sensor,
Tree: dbRepo.Tree,
Schema: dbRepo.Schema,
Copy link
Member

Choose a reason for hiding this comment

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

We dont need this

Copy link
Member

@choffmann choffmann left a comment

Choose a reason for hiding this comment

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

Please implement the feedback. I can't test the application because it won't build and there are some issues I mentioned in the review. If you have fixed them, I would like to do another review.

In general, you only need to work within the storage layer/directory, so the other layers shouldn't be updated.

@choffmann choffmann changed the title swithch from mangodb to postgreSQL feat: switch from mangodb to postgres Aug 3, 2024
@choffmann
Copy link
Member

I think we should implement this in #21. But work on that, we need to finish #7 first.

@choffmann choffmann closed this Aug 21, 2024
@choffmann choffmann deleted the postgreSQL branch October 13, 2024 15:23
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.

Switch to PostgreSQL Database
2 participants