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

Bugfix/fix post install error #141

Merged
merged 3 commits into from
Oct 15, 2024
Merged

Conversation

IonesioJunior
Copy link
Member

@IonesioJunior IonesioJunior commented Oct 14, 2024

This PR addresses the issue in the syftbox app install command related to the post-installation step. Previously, the post-install step executed commands based on the current working directory rather than the specific directory of the app being installed, as defined in config.json. This behavior caused inconsistencies when accessing app scripts and configurations. The PR modifies the implementation to ensure that the post-installation command now executes within the context of the app's installation path, guaranteeing that all app-specific scripts and configurations are correctly referenced and available during installation.

Changes

  • syftbox/app/install.py

Testing

You can test it by cloning this branch, installing the syftbox and running syftbox app install in a app repository that has a post_install pipeline defined. (eg: IonesioJunior/ring)

IonesioJunior and others added 2 commits October 14, 2024 17:51
…roper app path

Before, post_install command was running in the directory where syftbox app install is called
@IonesioJunior IonesioJunior requested a review from snwagh October 15, 2024 14:08
Copy link
Contributor

@snwagh snwagh left a comment

Choose a reason for hiding this comment

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

Looks good to me, haven't tested it though.

@IonesioJunior IonesioJunior merged commit ba440b4 into main Oct 15, 2024
7 checks passed
@IonesioJunior IonesioJunior deleted the bugfix/fix_post_install_error branch October 15, 2024 22:07
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.

3 participants