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

Implement basic insert support for SAP HANA #2732

Merged
merged 21 commits into from Jun 21, 2023
Merged

Conversation

polyal
Copy link
Contributor

@polyal polyal commented May 29, 2023

Description

Closes #{issue_number}.

Readiness checklist

  • I added/updated unit tests.
  • I added/updated integration/compatibility tests.
  • I added/updated comments and checked rendering.
  • I made spot refactorings.
  • I updated user documentation.
  • I ran task all, and it passed.
  • I ensured that PR title is good enough for the changelog.
  • (for maintainers only) I set Reviewers (@FerretDB/core), Labels, Project and project's Sprint fields.
  • I marked all done items in this checklist.

@polyal polyal requested a review from AlekSi as a code owner May 29, 2023 22:56
@mergify mergify bot assigned polyal May 29, 2023
@polyal
Copy link
Contributor Author

polyal commented May 29, 2023

I get this error with task lint locally.

internal/handlers/hana/hanadb/insert.go:21:2: github.com/FerretDB/FerretDB/internal/handlers/sjson is in the denylist (depguard)

Is there a way to suppress it? Or how come its in the deny list?

@codecov
Copy link

codecov bot commented May 29, 2023

Codecov Report

Merging #2732 (bbbb44a) into main (93b80a4) will decrease coverage by 60.09%.
The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #2732       +/-   ##
==========================================
- Coverage   64.25%   4.17%   -60.09%     
==========================================
  Files         450     447        -3     
  Lines       23125   23137       +12     
==========================================
- Hits        14860     966    -13894     
- Misses       7299   22098    +14799     
+ Partials      966      73      -893     
Impacted Files Coverage Δ
internal/handlers/hana/hanadb/collections.go 0.00% <0.00%> (ø)
internal/handlers/hana/hanadb/databases.go 0.00% <0.00%> (ø)
internal/handlers/hana/hanadb/insert.go 0.00% <0.00%> (ø)
internal/handlers/hana/msg_create.go 0.00% <0.00%> (ø)
internal/handlers/hana/msg_drop.go 0.00% <0.00%> (ø)
internal/handlers/hana/msg_dropdatabase.go 0.00% <0.00%> (ø)
internal/handlers/hana/msg_insert.go 0.00% <0.00%> (ø)
internal/handlers/hana/msg_listdatabases.go 0.00% <0.00%> (ø)

... and 263 files with indirect coverage changes

Flag Coverage Δ
integration 4.17% <0.00%> (-53.22%) ⬇️
mongodb 4.17% <0.00%> (-0.06%) ⬇️
pg ?
shard-1 4.17% <0.00%> (-37.31%) ⬇️
shard-2 ?
shard-3 ?
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Member

@AlekSi AlekSi left a comment

Choose a reason for hiding this comment

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

See

FerretDB/.golangci.yml

Lines 161 to 194 in 70d692c

# only `pg` handler can import `pgdb` package, no other handler can do that
- linters: [depguard]
path: internal/handlers/pg
text: pgdb
- linters: [depguard]
path: internal/handlers/registry
text: pgdb
- linters: [depguard]
path: cmd/envtool
text: pgdb
# only `pg` handler and `sqlite` backend can import `sjson` package, no other handlers or backends can do that
- linters: [depguard]
path: internal/handlers/pg
text: sjson
- linters: [depguard]
path: internal/backends/sqlite
test: sjson
# only `tigris` handler can import `tigrisdb` package, no other handler can do that
- linters: [depguard]
path: internal/handlers/tigris
text: tigrisdb
- linters: [depguard]
path: internal/handlers/registry
text: tigrisdb
- linters: [depguard]
path: cmd/envtool
text: tigrisdb
# only `tigris` handler can import `tjson` package, no other handler can do that
- linters: [depguard]
path: internal/handlers/tigris
text: tjson

@polyal polyal requested a review from AlekSi June 2, 2023 17:47
@mergify
Copy link
Contributor

mergify bot commented Jun 6, 2023

@polyal this pull request has merge conflicts.

@mergify mergify bot added the conflict PRs that have merge conflicts label Jun 6, 2023
@mergify mergify bot removed the conflict PRs that have merge conflicts label Jun 6, 2023
@AlekSi AlekSi added this to the Next milestone Jun 7, 2023
@AlekSi AlekSi added code/chore Code maintenance improvements trust PRs that can access Actions secrets labels Jun 7, 2023
Copy link
Member

@AlekSi AlekSi 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 listDatabases command as it is required for our tests

@polyal polyal requested review from AlekSi June 16, 2023 00:10
Copy link
Member

@AlekSi AlekSi left a comment

Choose a reason for hiding this comment

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

2023-06-16T23:59:57.985Z	INFO	setup/startup.go:87	Target system: ferretdb-hana (built-in).
2023-06-16T23:59:57.986Z	INFO	debug	debug/debug.go:95	Starting debug server on http://127.0.0.1:41195/
2023-06-16T23:59:57.990Z	INFO	setup/startup.go:98	Compat system: MongoDB (mongodb://127.0.0.1:47017/).
-test.shuffle 1686959997990741692
2023-06-17T00:00:00.122Z	INFO	slog/handler.go:119	ERROR DB conn read error conn=1 error=EOF "local address"=10.0.0.36:54396 "remote address"=3.214.124.25:443
2023-06-17T00:00:02.231Z	INFO	slog/handler.go:119	ERROR DB conn read error conn=2 error=EOF "local address"=10.0.0.36:37788 "remote address"=23.23.150.204:443
2023-06-17T00:00:04.255Z	INFO	slog/handler.go:119	ERROR DB conn read error conn=3 error=EOF "local address"=10.0.0.36:54410 "remote address"=3.214.124.25:443
--- FAIL: TestCommandsAdministrationServerStatusFreeMonitoring (9.26s)
    commands_administration_test.go:1019: [client.go:94 setup.makeClient] (InternalError) [msg_listdatabases.go:33 hana.(*Handler).MsgListDatabases] [hana.go:105 hana.(*Handler).DBPool] hanadb.NewPool: driver: bad connection: EOF
panic: setupClient: [client.go:94 setup.makeClient] (InternalError) [msg_listdatabases.go:33 hana.(*Handler).MsgListDatabases] [hana.go:105 hana.(*Handler).DBPool] hanadb.NewPool: driver: bad connection: EOF [recovered]
	panic: setupClient: [client.go:94 setup.makeClient] (InternalError) [msg_listdatabases.go:33 hana.(*Handler).MsgListDatabases] [hana.go:105 hana.(*Handler).DBPool] hanadb.NewPool: driver: bad connection: EOF [recovered]
	panic: setupClient: [client.go:94 setup.makeClient] (InternalError) [msg_listdatabases.go:33 hana.(*Handler).MsgListDatabases] [hana.go:105 hana.(*Handler).DBPool] hanadb.NewPool: driver: bad connection: EOF [recovered]
	panic: setupClient: [client.go:94 setup.makeClient] (InternalError) [msg_listdatabases.go:33 hana.(*Handler).MsgListDatabases] [hana.go:105 hana.(*Handler).DBPool] hanadb.NewPool: driver: bad connection: EOF [recovered]
	panic: setupClient: [client.go:94 setup.makeClient] (InternalError) [msg_listdatabases.go:33 hana.(*Handler).MsgListDatabases] [hana.go:105 hana.(*Handler).DBPool] hanadb.NewPool: driver: bad connection: EOF

Let's discuss on Slack or email how we could get a working database for testing

@AlekSi AlekSi removed this from the v1.4.0 milestone Jun 19, 2023
@AlekSi AlekSi added this to the Next milestone Jun 19, 2023
@AlekSi AlekSi changed the title Hana Insert Impl Basics Implement basic insert support for SAP HANA Jun 21, 2023
@AlekSi AlekSi merged commit 643b387 into FerretDB:main Jun 21, 2023
18 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code/chore Code maintenance improvements trust PRs that can access Actions secrets
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants