-
Notifications
You must be signed in to change notification settings - Fork 55
Add toy example for Dask scheduler with remote worker and local Postgres #843
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a toy example test script to demonstrate the integration of a local Dask scheduler with a remote Dask worker and a local PostgreSQL database for storing computation results.
- Establishes an SSH tunnel to forward necessary ports
- Launches the Dask scheduler and a remote worker for distributed computations
- Sets up and initializes a PostgreSQL table, performs computations, and verifies results via database queries
try: | ||
p.terminate() | ||
p.wait(timeout=5) | ||
except: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid using a bare 'except:' clause; specify 'except Exception:' to prevent inadvertently catching unexpected exceptions.
except: | |
except Exception: |
Copilot uses AI. Check for mistakes.
'port': db_port, | ||
'database': 'postgres', | ||
'user': 'postgres', | ||
'password': '0504' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded database credentials can pose a security risk; consider externalizing sensitive configuration data.
'password': '0504' | |
'password': os.getenv('DB_PASSWORD', 'default_password') # Use environment variable for password |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some minor syle comments.
username = "zliu890" | ||
server = "eagle.cc.gatech.edu" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use argparse for these two?
except Exception as e: | ||
print(f"Failed to retrieve results from database: {e}") | ||
|
||
# 1. Establish SSH tunnel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
put all this under a if __name__ == '__main__':
@@ -0,0 +1,227 @@ | |||
import subprocess |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we use a file-level docstring as well. you can find examples in other gtsfm files.
Also this file would be better located in the scripts folder instead of the tests folder. the tests folder is for unit tests. https://github.com/borglab/gtsfm/tree/master/scripts
Add Toy example to test Dask distributed computing with remote worker and local PostgreSQL
Description
This PR adds a toy example test script that demonstrates the integration between local Dask scheduler and PostgreSQL and remote Dask worker in GTSFM's eagle cluster. The test verifies the following functionality:
Test Details
The test script (
test_local_scheduler_remote_worker_local_postgres.py
) performs the following: