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

Push/Pull/Fetch while sql-server is running #6460

Closed
styledev opened this issue Aug 4, 2023 · 6 comments
Closed

Push/Pull/Fetch while sql-server is running #6460

styledev opened this issue Aug 4, 2023 · 6 comments
Assignees
Labels

Comments

@styledev
Copy link

styledev commented Aug 4, 2023

It would be amazing if we can push/pull/fetch from the command line while the dolt sql-server is running.

@timsehn timsehn added enhancement New feature or request cli labels Aug 4, 2023
@timsehn
Copy link
Contributor

timsehn commented Aug 4, 2023

Work around for this is

dolt sql -q "call dolt_push()"
dolt sql -q "call dolt_pull()"
dolt sql -q "call dolt_fetch()"

@ricardoreiter
Copy link

@timsehn doesn't work in case of a hosted dolt instance as you can't pass the remoteapi creds

@macneale4 macneale4 self-assigned this Aug 22, 2023
@macneale4
Copy link
Contributor

@ricardoreiter - I've been running up against the authentication issues of these methods, and I believe there is an inherent security concern which needs to be addressed before we tackle these methods. The issue is pretty fundamental to how we address this, and I believe getting this working securely in hosted is a heavy lift.

The first step to implementing these functions will actually disable the use of these CLI commands for any invocation which is off host - which doesn't actually address your concern. The first iteration of these methods will enable the CLI to run seamlessly when there is a local server running - with the caveat that the DOLT_REMOTE_PASSWORD used will be from the environment of the local server. This isn't a security risk because the CLI and the sql-server are run with the same admin permissions.

@timsehn
Copy link
Contributor

timsehn commented Sep 11, 2023

More explicitly.

Before doing this make sure you are dolt logined to DoltHub so you have write permissions to your database.

Here is the repro:

  1. Created this on DoltHub: https://www.dolthub.com/repositories/timsehn/test_push_pull_running_server/data/main

  2. Clone it locally and start a SQL server in one shell:

$ dolt clone timsehn/test_push_pull_running_server
cloning https://doltremoteapi.dolthub.com/timsehn/test_push_pull_running_server
$ cd test_push_pull_running_server 
$ dolt sql-server
Starting server with Config HP="localhost:3306"|T="28800000"|R="false"|L="info"|S="/tmp/mysql.sock"
  1. Try to dolt pull from the command line and sql interface:
$ dolt pull
database locked by another sql-server; either clone the database to run a second server, or delete the '/Users/timsehn/dolthub/dolt/test_push_pull_running_server/.dolt/sql-server.lock' if no other sql-servers are active
$ dolt sql -q "call dolt_pull()"
+--------------+-----------+
| fast_forward | conflicts |
+--------------+-----------+
| 1            | 0         |
+--------------+-----------+

Get lock file error using the CLI but using sql interface works. The CLI should work.

  1. Try dolt push from the command line and sql interface:
$ dolt push                                       
Everything up-to-date

With no updates the first push succeeds from the command line because it doesn't write a new manifest.

  1. Make an update and try and push it.
$ dolt sql -q "insert into tablename values (0, 'foo')"

$ dolt commit -am "Inserted values"
commit 1573cvkgml1d24k1t8c8ho8h2rrgrqpv (HEAD -> main) 
Author: timsehn <tim@dolthub.com>
Date:  Mon Sep 11 12:02:47 -0700 2023

        Inserted values

$ dolt push                                            
Uploaded 832 B of 832 B @ 6.73 kB/s.
error: push failed
cause: unknown push error; cannot update manifest: database is read only
$ dolt sql -q "call dolt_push()"                       
+---------+
| success |
+---------+
| 1       |
+---------+

$

Fails from the command line dolt push because it has a concurrency problem updating the manifest. This should work. At the very least it should fail with the same lock file error as dolt pull. Push from the SQL interface succeeds as intended.

@timsehn
Copy link
Contributor

timsehn commented Sep 11, 2023

We should fix the DoltHub case first because it is easier, then make it work with Hosted Dolt or other remotes.

@stephkyou
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants