Skip to content

Conversation

@lavesh00
Copy link

@lavesh00 lavesh00 commented Nov 1, 2025

Fixes #321

This PR adds automated Git pull functionality to the Ubuntu installation script:

Features:

  • Standalone update script (install/update.sh) for safe updates
  • Enhanced install.sh to detect existing installations and pull updates
  • Optional automated daily updates via systemd timer
  • Comprehensive documentation with update instructions
  • Supports multiple installations with dependency management

Changes:

  • Added install/update.sh: Interactive update script with backup, dependency checks, and service restart
  • Modified install/install.sh: Detects existing installations, prompts for update, sets up automated updates
  • Updated install/README.md: Added complete update section with 3 methods and troubleshooting

Testing:

  • Scripts validated for syntax and logic flow
  • Ready for testing on Linux environments

Summary by cubic

Adds automated Git pull updates to the Ubuntu installer and a safe, standalone update script to keep OpenAlgo installations current with minimal effort. Optional daily updates can be enabled via systemd for hands-free maintenance.

  • New Features
    • Added install/update.sh for interactive, safe updates (backup, auto-stash, dependency refresh via uv, service restart), with support for multiple installations.
    • Enhanced install/install.sh to detect existing installs and offer in-place updates, skipping re-clone when updating.
    • Introduced per-installation systemd timer and wrapper script for unattended daily updates.
    • Expanded README with three update methods and troubleshooting guidance.

Written for commit f76398c. Summary will update automatically on new commits.

Fixes marketcalls#321

This PR adds automated Git pull functionality to the Ubuntu installation script:

**Features:**
- Standalone update script (install/update.sh) for safe updates
- Enhanced install.sh to detect existing installations and pull updates
- Optional automated daily updates via systemd timer
- Comprehensive documentation with update instructions
- Supports multiple installations with dependency management

**Changes:**
- Added install/update.sh: Interactive update script with backup, dependency checks, and service restart
- Modified install/install.sh: Detects existing installations, prompts for update, sets up automated updates
- Updated install/README.md: Added complete update section with 3 methods and troubleshooting

**Testing:**
- Scripts validated for syntax and logic flow
- Ready for testing on Linux environments
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

5 issues found across 3 files

Prompt for AI agents (all 5 issues)

Understand the root cause of the following 5 issues and fix them.


<file name="install/update.sh">

<violation number="1" location="install/update.sh:262">
`local` cannot be used at top level. Replace it with a regular variable assignment so the multi-installation selection works.</violation>
</file>

<file name="install/README.md">

<violation number="1" location="install/README.md:380">
When multiple deployments exist, this wildcard expands to several update.sh files. Because install/update.sh treats the first argument as the deployment root (`SPECIFIC_PATH=&quot;$1&quot;`), the command ends up passing another update.sh file instead of a deployment directory and the script aborts without updating anything. Replace the wildcard with the actual deployment path when invoking the script so it can autodetect the rest.</violation>

<violation number="2" location="install/README.md:383">
Because the wildcard expands to additional update.sh paths when multiple deployments exist, the script receives the wrong `$1` value, fails its deployment-path check, and aborts. Point the command directly at the intended deployment’s update.sh instead of using `*` so the first argument remains the target base path.</violation>
</file>

<file name="install/install.sh">

<violation number="1" location="install/install.sh:642">
`git stash pop` here can pop the wrong stash entry when other stashes exist (manual or older auto-stashes), which can unexpectedly reapply unrelated changes. Please capture the stash ref you create and pop that specific entry instead of popping whatever is on top of the stack.</violation>

<violation number="2" location="install/install.sh:1153">
Because the heredoc delimiter isn’t quoted, `$(date)` is evaluated while generating the wrapper script, so the timer logs a fixed timestamp instead of the current time. Please escape the `$` (e.g. `\$(date)`) on this line and the other `$(date)` echoes in the heredoc.</violation>
</file>

Since this is your first cubic review, here's how it works:

  • cubic automatically reviews your code and comments on bugs and improvements
  • Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
  • Ask questions if you need clarification on any suggestion

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

# Multiple installations
log_message "\nFound ${#INSTALLATIONS[@]} OpenAlgo installation(s):" "$BLUE"
for i in "${!INSTALLATIONS[@]}"; do
local deploy_name=$(basename "${INSTALLATIONS[$i]}")
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 1, 2025

Choose a reason for hiding this comment

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

local cannot be used at top level. Replace it with a regular variable assignment so the multi-installation selection works.

Prompt for AI agents
Address the following comment on install/update.sh at line 262:

<comment>`local` cannot be used at top level. Replace it with a regular variable assignment so the multi-installation selection works.</comment>

<file context>
@@ -0,0 +1,293 @@
+    # Multiple installations
+    log_message &quot;\nFound ${#INSTALLATIONS[@]} OpenAlgo installation(s):&quot; &quot;$BLUE&quot;
+    for i in &quot;${!INSTALLATIONS[@]}&quot;; do
+        local deploy_name=$(basename &quot;${INSTALLATIONS[$i]}&quot;)
+        log_message &quot;  [$((i+1))] $deploy_name (${INSTALLATIONS[$i]})&quot; &quot;$BLUE&quot;
+    done
</file context>
Suggested change
local deploy_name=$(basename "${INSTALLATIONS[$i]}")
deploy_name=$(basename "${INSTALLATIONS[$i]}")

✅ Addressed in f76398c


```bash
# Update all installations interactively
sudo bash /var/python/openalgo-flask/*/openalgo/install/update.sh
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 1, 2025

Choose a reason for hiding this comment

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

When multiple deployments exist, this wildcard expands to several update.sh files. Because install/update.sh treats the first argument as the deployment root (SPECIFIC_PATH="$1"), the command ends up passing another update.sh file instead of a deployment directory and the script aborts without updating anything. Replace the wildcard with the actual deployment path when invoking the script so it can autodetect the rest.

Prompt for AI agents
Address the following comment on install/README.md at line 380:

<comment>When multiple deployments exist, this wildcard expands to several update.sh files. Because install/update.sh treats the first argument as the deployment root (`SPECIFIC_PATH=&quot;$1&quot;`), the command ends up passing another update.sh file instead of a deployment directory and the script aborts without updating anything. Replace the wildcard with the actual deployment path when invoking the script so it can autodetect the rest.</comment>

<file context>
@@ -367,12 +367,159 @@ After installation completes, verify each deployment:
+
+```bash
+# Update all installations interactively
+sudo bash /var/python/openalgo-flask/*/openalgo/install/update.sh
+
+# Update a specific installation
</file context>
Suggested change
sudo bash /var/python/openalgo-flask/*/openalgo/install/update.sh
sudo bash /var/python/openalgo-flask/yourdomain-broker/openalgo/install/update.sh

✅ Addressed in f76398c

sudo bash /var/python/openalgo-flask/*/openalgo/install/update.sh
# Update a specific installation
sudo bash /var/python/openalgo-flask/*/openalgo/install/update.sh /var/python/openalgo-flask/yourdomain-broker
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 1, 2025

Choose a reason for hiding this comment

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

Because the wildcard expands to additional update.sh paths when multiple deployments exist, the script receives the wrong $1 value, fails its deployment-path check, and aborts. Point the command directly at the intended deployment’s update.sh instead of using * so the first argument remains the target base path.

Prompt for AI agents
Address the following comment on install/README.md at line 383:

<comment>Because the wildcard expands to additional update.sh paths when multiple deployments exist, the script receives the wrong `$1` value, fails its deployment-path check, and aborts. Point the command directly at the intended deployment’s update.sh instead of using `*` so the first argument remains the target base path.</comment>

<file context>
@@ -367,12 +367,159 @@ After installation completes, verify each deployment:
+sudo bash /var/python/openalgo-flask/*/openalgo/install/update.sh
+
+# Update a specific installation
+sudo bash /var/python/openalgo-flask/*/openalgo/install/update.sh /var/python/openalgo-flask/yourdomain-broker
+```
+
</file context>
Suggested change
sudo bash /var/python/openalgo-flask/*/openalgo/install/update.sh /var/python/openalgo-flask/yourdomain-broker
sudo bash /var/python/openalgo-flask/yourdomain-broker/openalgo/install/update.sh /var/python/openalgo-flask/yourdomain-broker

✅ Addressed in f76398c

REMOTE_COMMIT=\$(sudo -u $WEB_USER git rev-parse origin/\${CURRENT_BRANCH})
if [ "\$CURRENT_COMMIT" != "\$REMOTE_COMMIT" ]; then
echo "[$(date)] Updating $DEPLOY_NAME from \${CURRENT_COMMIT:0:8} to \${REMOTE_COMMIT:0:8}..."
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 1, 2025

Choose a reason for hiding this comment

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

Because the heredoc delimiter isn’t quoted, $(date) is evaluated while generating the wrapper script, so the timer logs a fixed timestamp instead of the current time. Please escape the $ (e.g. \$(date)) on this line and the other $(date) echoes in the heredoc.

Prompt for AI agents
Address the following comment on install/install.sh at line 1153:

<comment>Because the heredoc delimiter isn’t quoted, `$(date)` is evaluated while generating the wrapper script, so the timer logs a fixed timestamp instead of the current time. Please escape the `$` (e.g. `\$(date)`) on this line and the other `$(date)` echoes in the heredoc.</comment>

<file context>
@@ -1073,3 +1125,119 @@ log_message &quot;Restart OpenAlgo: sudo systemctl restart $SERVICE_NAME&quot; &quot;$BLUE&quot;
+REMOTE_COMMIT=\$(sudo -u $WEB_USER git rev-parse origin/\${CURRENT_BRANCH})
+
+if [ &quot;\$CURRENT_COMMIT&quot; != &quot;\$REMOTE_COMMIT&quot; ]; then
+    echo &quot;[$(date)] Updating $DEPLOY_NAME from \${CURRENT_COMMIT:0:8} to \${REMOTE_COMMIT:0:8}...&quot;
+    
+    # Stash any local changes
</file context>
Suggested change
echo "[$(date)] Updating $DEPLOY_NAME from \${CURRENT_COMMIT:0:8} to \${REMOTE_COMMIT:0:8}..."
echo "[\$(date)] Updating $DEPLOY_NAME from \${CURRENT_COMMIT:0:8} to \${REMOTE_COMMIT:0:8}..."

✅ Addressed in f76398c

# Restore stashed changes if any
if sudo -u $WEB_USER git stash list | grep -q "Auto-stash before install update"; then
log_message "Restoring stashed changes..." "$YELLOW"
sudo -u $WEB_USER git stash pop 2>/dev/null || log_message "Warning: Could not restore stashed changes" "$YELLOW"
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 1, 2025

Choose a reason for hiding this comment

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

git stash pop here can pop the wrong stash entry when other stashes exist (manual or older auto-stashes), which can unexpectedly reapply unrelated changes. Please capture the stash ref you create and pop that specific entry instead of popping whatever is on top of the stack.

Prompt for AI agents
Address the following comment on install/install.sh at line 642:

<comment>`git stash pop` here can pop the wrong stash entry when other stashes exist (manual or older auto-stashes), which can unexpectedly reapply unrelated changes. Please capture the stash ref you create and pop that specific entry instead of popping whatever is on top of the stack.</comment>

<file context>
@@ -602,18 +602,70 @@ if ! command -v certbot &gt;/dev/null 2&gt;&amp;1; then
+        # Restore stashed changes if any
+        if sudo -u $WEB_USER git stash list | grep -q &quot;Auto-stash before install update&quot;; then
+            log_message &quot;Restoring stashed changes...&quot; &quot;$YELLOW&quot;
+            sudo -u $WEB_USER git stash pop 2&gt;/dev/null || log_message &quot;Warning: Could not restore stashed changes&quot; &quot;$YELLOW&quot;
+        fi
+        
</file context>

✅ Addressed in f76398c

- update.sh: remove invalid top-level 'local' for multi-install selection
- install.sh: escape 11/05/2025 14:29:13 inside heredoc-generated scripts for live timestamps
- install.sh: capture and pop specific stash ref to avoid popping wrong entry
- README: replace wildcard update examples with explicit deployment path
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 3 files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement GTT Order Support

1 participant