Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2497 +/- ##
==========================================
- Coverage 79.23% 79.19% -0.05%
==========================================
Files 670 670
Lines 54182 54225 +43
Branches 734 734
==========================================
+ Hits 42930 42941 +11
- Misses 11172 11204 +32
Partials 80 80
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jmthomas
left a comment
There was a problem hiding this comment.
Makes sense. What does this actually look like in the gemspec? Something like:
s.openc3_cosmos_minimim_version = '6.0.0'
Does it support a version like 6.0 or just 6?
|
@jmthomas You set it with the other app store properties like this: s.metadata = {
"openc3_cosmos_minimum_version" => "6.9.0",
"openc3_store_title" => "Video Player",
"openc3_store_description" => "This plugin provides COSMOS with an integrated video player capable of playing back most common video file types, as well as HLS streams. Included is the top-level tool and a widget that can be used in TlmViewer screens.",
"openc3_store_keywords" => "video, streaming",
"openc3_store_image" => "public/store_img.png"
}It has to be a valid semver string, so you have to specify all three digits (6.0.0). It does support the |
|
Do not merge until after the next App Store deployment (ETA: 11/24 - 11/26). The app store changes that this depends on have been merged, but the infra needed for their prod deployment has not been set up yet. |
| original_filename = File.basename(store_data['gem_url']) | ||
|
|
||
| # Try to find the correct hostname (in case it's localhost and needs to be host.docker.internal) | ||
| adjusted_gem_url = check_localhost_reachability(store_data['gem_url'], params[:store_id]) |
There was a problem hiding this comment.
Why would a app store author publish a gem with a URL of localhost? That doesn't really make sense right? Don't we require these to be available at Rubygems or some internal repo like artifactory?
There was a problem hiding this comment.
Being able to self-host the store is one of its requirements, so gems might be at localhost
5d1a022 to
d7bc517
Compare
d7bc517 to
58363ce
Compare
|
Added API key support for Enterprise/allowlist plugins and caught this up with recent app store changes. Ready for re-review |
jmthomas
left a comment
There was a problem hiding this comment.
Changes look good but I'm not sure why I'm getting the "Plugin Store Error" when I try to connect.
| api_key_setting = OpenC3::SettingModel.get(name: 'store_api_key', scope: 'DEFAULT') | ||
| api_key = api_key_setting['data'] if api_key_setting | ||
|
|
||
| test_url = "http://#{uri.host}:#{uri.port}/api/v1.1/cosmos_plugins/#{store_id}" |
There was a problem hiding this comment.
This is for deploying your own app store? How / where will this be documented?
There was a problem hiding this comment.
It will be documented when we open source the app store
|
|
||
| const settingName = 'store_url' | ||
| const SETTING_NAME = 'store_url' | ||
| const DEFAULT_STORE_URL = 'https://store.openc3.com' |
There was a problem hiding this comment.
When I run this branch I get the following when browsing the app store: Error contacting plugin store at https://store.openc3.com (status: 404)
There was a problem hiding this comment.
Works now that the app store is launched
| image_url: String, // Set for app store plugins | ||
| licenses: Array, | ||
| // rating: Number, | ||
| // downloads: Number, |
There was a problem hiding this comment.
What is with all the commented out fields here?
There was a problem hiding this comment.
Features that were in the proof of concept, and will be implemented for real later
| settings with the | ||
| <v-icon icon="mdi-cog" size="small" /> icon and paste your API key into | ||
| the API key field. | ||
| </v-card-text> |
There was a problem hiding this comment.
Is this all implemented in the App Store?
| 'name' => @name, | ||
| 'variables' => @variables, | ||
| 'plugin_txt_lines' => @plugin_txt_lines, | ||
| 'minimum_cosmos_version' => @minimum_cosmos_version, |
There was a problem hiding this comment.
Does this need a migration to support adding this field or I assume if it is nil we just ignore it.
There was a problem hiding this comment.
If it's nil, we ignore it (assume minimum version of 5.0.0)
d29b01c to
34c4275
Compare
6952b5b to
95ab110
Compare
See also: https://github.com/OpenC3/app-store/pull/41