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

Initial DB Table #1213

Merged
merged 39 commits into from
Oct 4, 2024
Merged

Conversation

ralikio
Copy link
Member

@ralikio ralikio commented Sep 27, 2024

Description

Changes proposed in this pull request:

  • Initial Schema for Kyma Bindings,
  • go structs and logic.

Related issue(s)

#1198 #284

@kyma-bot kyma-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 27, 2024
@kyma-bot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@ralikio ralikio added kind/enhancement Categorizes issue or PR as related to modifying or improving an existing feature and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Sep 27, 2024
@kyma-bot kyma-bot added the cla: yes Indicates the PR's author has signed the CLA. label Sep 27, 2024
Copy link

Add one of following labels

- kind/feature -> Use it when you want to submit a new feature

- kind/enhancement -> Use it when you modify or improve an existing feature

- kind/bug -> Use it when you fix a bug

@kyma-bot kyma-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 27, 2024
@kyma-bot kyma-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 2, 2024
@@ -20,6 +20,7 @@ const (
SubaccountStatesTableName = "subaccount_states"
CreatedAtField = "created_at"
InstancesArchivedTableName = "instances_archived"
BindingssTableName = "bindings"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
BindingssTableName = "bindings"
BindingsTableName = "bindings"

@@ -21,6 +21,37 @@ type writeSession struct {
transaction *dbr.Tx
}

func (ws writeSession) DeleteBinding(ID string) dberr.Error {
_, err := ws.deleteFrom(BindingssTableName).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
_, err := ws.deleteFrom(BindingssTableName).
_, err := ws.deleteFrom(BindingsTableName).

return dberr.AlreadyExists("binding with id %s already exist for runtime %s", binding.ID, binding.RuntimeID)
}
}
return dberr.Internal("Failed to insert record to Instance table: %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return dberr.Internal("Failed to insert record to Instance table: %s", err)
return dberr.Internal("Failed to insert record to Binding table: %s", err)

ExpireationSeconds int64
GenerationMethod string

Version int
Copy link
Contributor

Choose a reason for hiding this comment

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

YAGNI?

CreatedAt: binding.CreatedAt,
UpdatedAt: binding.UpdatedAt,
ExpiredAt: binding.ExpiredAt,
Version: binding.Version,
Copy link
Contributor

Choose a reason for hiding this comment

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

Not present in the DB structure yet.

return binding, nil
}

func (r readSession) ListBindings(runtimeID string) ([]dbmodel.BindingDTO, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Access not by index (yet)

@piotrmiskiewicz
Copy link
Member

consider adding Instance ID

return sess.DeleteBinding(instanceID)
}

func (s *Binding) List(instanceID string) ([]internal.Binding, error) {
Copy link
Member

Choose a reason for hiding this comment

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

ListByInstanceID

@kyma-bot kyma-bot added the lgtm Looks good to me! label Oct 4, 2024
@kyma-bot kyma-bot removed the lgtm Looks good to me! label Oct 4, 2024
piotrmiskiewicz
piotrmiskiewicz previously approved these changes Oct 4, 2024
@kyma-bot kyma-bot added the lgtm Looks good to me! label Oct 4, 2024
@kyma-bot kyma-bot removed the lgtm Looks good to me! label Oct 4, 2024
piotrmiskiewicz
piotrmiskiewicz previously approved these changes Oct 4, 2024
@kyma-bot kyma-bot added the lgtm Looks good to me! label Oct 4, 2024
@kyma-bot kyma-bot removed the lgtm Looks good to me! label Oct 4, 2024
@kyma-bot kyma-bot added the lgtm Looks good to me! label Oct 4, 2024
@kyma-gopher-bot kyma-gopher-bot merged commit 1947428 into kyma-project:main Oct 4, 2024
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Indicates the PR's author has signed the CLA. kind/enhancement Categorizes issue or PR as related to modifying or improving an existing feature lgtm Looks good to me! size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants