-
Notifications
You must be signed in to change notification settings - Fork 32
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
Fix pypi url usage #1226
Fix pypi url usage #1226
Conversation
Quality Gate passedIssues Measures |
@@ -36,7 +36,7 @@ module OpenC3 | |||
# and destroy to allow interaction with gem files from the PluginModel and | |||
# the GemsController. | |||
class GemModel | |||
include Api | |||
extend Api |
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.
Doesn't extend add as class methods vs include adds as instance methods
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.
Yep. That was the bug as we were using get_setting in a class method not an instance method.
end | ||
Logger.info "Installing python package: #{name_or_path}" | ||
result = OpenC3::ProcessManager.instance.spawn(["pip", "install", "--user", "-i", pypi_url, package_file_path], "package_install", package_filename, Time.now + 3600.0, scope: scope) | ||
result = OpenC3::ProcessManager.instance.spawn(["/openc3/bin/pipinstall", "--user", "--no-warn-script-location", "-i", pypi_url, package_file_path], "package_install", package_filename, Time.now + 3600.0, scope: scope) |
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.
Looks like we were using this in the plugin_model.rb but not here. Is this the real fix?
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.
This adds a retry if the remote install fails, that try to just do local.
pypi_url += '/simple' | ||
end | ||
pypi_url ||= 'https://pypi.org/simple' | ||
end |
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.
Logic is cleaner but basically the same?
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.
Basically the same.
closes #1137