Skip to content

Commit

Permalink
Merge pull request #367 from OpenMined/fixes
Browse files Browse the repository at this point in the history
many bug fixes
  • Loading branch information
yashgorana authored Nov 11, 2024
2 parents a6dcded + 5484fae commit 810597f
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 145 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/e2e-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,11 @@ jobs:
just reset
just test-e2e ${{ matrix.e2e-test }}
- name: Cleanup macOS Icon\r Files
- name: Cleanup unnecessary files
if: ${{ failure() && startsWith(matrix.runner, 'sh-macos') }}
run: |
find . -type f -name "Icon*" -exec rm -f {} \;
find . -type f -name "syftbox.pid" -exec rm -f {} \;
- name: Upload logs & client/server state
if: failure()
Expand Down
5 changes: 0 additions & 5 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,12 @@ description = "Add your description here"
readme = "README.md"
requires-python = ">=3.9"
dependencies = [
"cryptography>=43.0.1",
"requests>=2.32.3",
"fastapi>=0.114.0",
"uvicorn>=0.30.6",
"apscheduler>=3.10.4",
"jinja2>=3.1.4",
"typing-extensions>=4.12.2",
"sqlalchemy>=2.0.34",
"markdown>=3.7",
"pandas>=2.2.2",
"setuptools>=75.1.0",
"postmarker>=1.0",
"watchdog>=5.0.2",
"pydantic-settings>=2.5.2",
Expand Down
6 changes: 3 additions & 3 deletions syftbox/client/client2.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,17 +247,17 @@ def run_migration(config: SyftClientConfig):

# create the datasites directory & move all under it
for dir in old_sync_folder.glob("*@*"):
dir.rename(new_ws.datasites / dir.name)
shutil.move(str(dir), str(new_ws.datasites))

# move syftignore file
old_ignore_file = old_sync_folder / IGNORE_FILENAME
if old_ignore_file.exists():
old_ignore_file.rename(new_ws.datasites / IGNORE_FILENAME)
shutil.move(str(old_ignore_file), str(new_ws.datasites / IGNORE_FILENAME))

# move old sync state file
old_sync_state = old_sync_folder / ".syft" / "local_syncstate.json"
if old_sync_state.exists():
old_sync_state.rename(new_ws.plugins / "local_syncstate.json")
shutil.move(str(old_sync_state), str(new_ws.plugins / "local_syncstate.json"))
if old_sync_state.parent.exists():
shutil.rmtree(str(old_sync_state.parent))

Expand Down
4 changes: 2 additions & 2 deletions syftbox/client/utils/error_reporting.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import sys
from platform import platform

import requests
import httpx
from pydantic import BaseModel, Field
from typing_extensions import Optional

Expand Down Expand Up @@ -35,6 +35,6 @@ def make_error_report(client_config: SyftClientConfig):
def try_get_server_version(server_url):
try:
# do not use the server_client here, as it may not be in bad state
return requests.get(f"{server_url}/info").json()["version"]
return httpx.get(f"{server_url}/info").json()["version"]
except Exception:
return None
4 changes: 3 additions & 1 deletion syftbox/lib/client_config.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import json
import os
import shutil
from pathlib import Path
from typing import Optional

Expand Down Expand Up @@ -107,7 +108,8 @@ def migrate(self) -> Self:
# if we loaded the legacy config, we need to move it to new config
if self.path.name == LEGACY_CONFIG_NAME:
new_path = Path(self.path.parent, DEFAULT_CONFIG_PATH.name)
self.path = self.path.rename(new_path)
shutil.move(str(self.path), str(new_path))
self.path = new_path
self.save()

return self
Expand Down
11 changes: 6 additions & 5 deletions syftbox/lib/logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,24 @@ def setup_logger(
logger.add(level=level, sink=sys.stderr, diagnose=False, backtrace=False)

# new file per run - no rotation needed
# always log debug level
log_file = Path(log_dir, f"syftbox_{int(datetime.now().timestamp())}.log")
logger.add(
log_file,
level=level,
level="DEBUG",
rotation=None,
compression=None,
)

# keep last 5 logs
logs_to_delete = sorted(log_dir.glob("syftbox_*.log"))[:-keep_logs]
for log in logs_to_delete:
log.unlink()
try:
log.unlink()
except Exception:
pass


def zip_logs(output_path, log_dir: PathLike = DEFAULT_LOGS_DIR):
logs_folder = to_path(log_dir)
return make_archive(output_path, "zip", logs_folder)


setup_logger()
23 changes: 13 additions & 10 deletions tests/e2e/ring/run.bash
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ RING_DIR=$E2E_DIR/ring
LOGS_DIR=$RING_DIR/logs
CLIENT_DIR=$RING_DIR/client
SERVER_DIR=$RING_DIR/server
CONFIG_DIR=$RING_DIR/config
DOMAIN="openmined.org"

########## Client & Server ##########
Expand All @@ -31,11 +30,12 @@ bg_start_client() {
info "Starting client email=$email port=$port"

run_bg syftbox client \
--config=$CONFIG_DIR/$user.json \
--config=$CLIENT_DIR/$user/config.json \
--data-dir=$CLIENT_DIR/$user \
--email=$email \
--port=$port \
--no-open-dir \
--verbose \
--server=http://localhost:5001 &> $LOGS_DIR/client.$1.log
}

Expand All @@ -53,40 +53,43 @@ bg_start_syftbox() {

e2e_prepare_dirs() {
rm -rf $E2E_DIR
mkdir -p $LOGS_DIR $CONFIG_DIR $CLIENT_DIR
mkdir -p $LOGS_DIR $CLIENT_DIR
}

########## Ring ##########

path_user_datasite() {
# get path to client's datasite
local user=$1
echo "$CLIENT_DIR/$user/datasites/$user@$DOMAIN"
}

path_ring_app() {
# get path to client's ring app

echo "$CLIENT_DIR/$1/apps/ring"
local user=$1
echo "$CLIENT_DIR/$user/apps/ring"
}

path_ring_pipeline() {
# get path to client's ring app pipeline

echo "$CLIENT_DIR/$1/$1@$DOMAIN/app_pipelines/ring"
local user=$1
echo "$(path_user_datasite $user)/app_pipelines/ring"
}

wait_for_ring_app() {
# wait for ring app to be be installed

for user in $@
do wait_for_path "$(path_ring_app $user)/run.sh" 30
done
}

ring_copy_secrets() {
# copy secret.json to ring app

for user in $@
do cp $SCRIPT_DATA_DIR/$user.secret.json "$(path_ring_app $user)/secret.json"
done
}


ring_init() {
# place data.json in apps_pipelines/ring/running

Expand Down
Loading

0 comments on commit 810597f

Please sign in to comment.