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

MM-19445 - Added version information to plugin diagnostics data #13408

Merged
merged 31 commits into from Jan 16, 2020

Conversation

marianunez
Copy link
Member

@marianunez marianunez commented Dec 17, 2019

Summary

Within the config_plugin data structure:

  • Included entries to track version number for whitelisted plugins under version-[pluginName]
  • Added new entry for EnableRemoteMarketplace setting.
  • Added webex to the list of whitelisted plugins for enabled and version number.

Ticket Link

MM-19445

alifarooq0 and others added 8 commits November 18, 2019 22:16
* MM-19612 - Support querying local plugin marketplace when upstream unavailable or disabled

* Update translations file

* Fixed comment

* Updated to check EnableRemoteMarketplace setting and LocalOnly to get marketplace plugins

* Fixed unit tests

* Tests cleanup code

* Removed unused error message

* Updated tests
@marianunez marianunez added 2: Dev Review Requires review by a developer 1: PM Review Requires review by a product manager labels Dec 17, 2019
@marianunez marianunez added this to the v5.20.0 milestone Dec 17, 2019
@marianunez
Copy link
Member Author

marianunez commented Dec 17, 2019

@aaronrothschild @jasonblais data structure and entry name (enable-[pluginName]) open to change for what would be best for reporting purposes in segment.

If possible, I would suggest to changing the entry name to just the [pluginName] leaving room to expand that for future data points as needed.

@aaronrothschild
Copy link
Contributor

@aaronrothschild @jasonblais data structure and entry name (enable-[pluginName]) open to change for what would be best for reporting purposes in segment.

If possible, I would suggest to changing the entry name to just the [pluginName] leaving room to expand that for future data points as needed.

Thanks Maria! I'll leave it to @jasonblais regarding the data structure that he's looking into. Looks good otherwise, looking forward to seeing the data! :)

app/diagnostics.go Outdated Show resolved Hide resolved
app/diagnostics.go Outdated Show resolved Hide resolved
marianunez and others added 3 commits December 17, 2019 11:56
* consume prepackaged plugins into memory

* missing i18n

* remove spurious .gitignore changes

* return on failure to install prepackged plugins

* cleanup

* s/plugins/availablePlugins

* whitespace

* don't return extractDir when not needed

* s/plug/plugin

* error on icon, cleanup

* update armored version of testplugin signature

* honour AutomaticPrepackagedPlugins

* document getPrepackagedPlugin
Copy link
Member

@lieut-data lieut-data left a comment

Choose a reason for hiding this comment

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

Thanks, @marianunez! Two thoughts below re: how we want to organize and structure the data for upstream processing.

app/diagnostics.go Outdated Show resolved Hide resolved
app/diagnostics.go Outdated Show resolved Hide resolved
marianunez and others added 2 commits December 20, 2019 13:06
* Added prepackaged plugins to marketplace results

* PR Feedback

* PR Feedback

* Update error where definition

* Removing unnecessary var declaration

* Updated comments
* MM-21263 - Use EnableRemoteMarketplace in marketplace install endpoint

* Call updateConfig before calling NewServer in TestHelper

* Added translations

* PR feedback

* Translations

* Feedback

* s/helpers.go/download.go

* Converging env.PrepackagedPlugins
@marianunez
Copy link
Member Author

I've updated this PR to the changes we agreed on keeping the version numbers in their separate entries for each plugin. It is now ready for re-review. 👍

@hanzei
Copy link
Contributor

hanzei commented Jan 13, 2020

@aaronrothschild Heads up that https://docs.mattermost.com/administration/telemetry.html needs an update after this PR is merged.

app/diagnostics.go Show resolved Hide resolved
app/diagnostics.go Show resolved Hide resolved
app/diagnostics.go Show resolved Hide resolved
app/diagnostics.go Show resolved Hide resolved
Copy link
Contributor

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

The code looks fine. I'm leaving the decision about the set of plugins to send diagnostics for to @aaronrothschild

@hanzei hanzei removed the 2: Dev Review Requires review by a developer label Jan 14, 2020
@lieut-data lieut-data added the Tests/Not Needed New release tests not required label Jan 14, 2020
@lieut-data
Copy link
Member

@aaronrothschild to confirm subset and help with post-merge testing.

@marianunez marianunez changed the base branch from mm-19606-feature-rework-prepackaged-plugins to master January 16, 2020 02:59
@marianunez
Copy link
Member Author

@aaronrothschild I will be merging this to meet v5.20 feature complete. Please review the list of plugins and let me know if there is any change to be done post-merge.

@marianunez marianunez merged commit c21ea5b into master Jan 16, 2020
@marianunez marianunez deleted the MM-19445 branch January 16, 2020 13:46
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1: PM Review Requires review by a product manager Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Tests/Not Needed New release tests not required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants