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.

Sign up for GitHub

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

Jump to bottom

Update to write Plugin manifest to dynamo and s3 #982

Merged
merged 17 commits into from
May 18, 2023
Merged

Update to write Plugin manifest to dynamo and s3 #982

merged 17 commits into from
May 18, 2023

Conversation

manasaV3
Copy link
Collaborator

@manasaV3 manasaV3 commented Apr 3, 2023 •
edited
Loading

Relates to #862

Description

  • In addition to s3, writes new manifest data to dynamo
  • If data already exists in s3 ensures the data is also present in dynamo, if not, it copies over the data from s3 to dynamo

@manasaV3 manasaV3 marked this pull request as ready for review April 4, 2023 21:31
Base automatically changed from plugins-clean-up to main April 4, 2023 21:39
@manasaV3 manasaV3 changed the base branch from main to plugin-db-infra April 4, 2023 21:44
@manasaV3 manasaV3 changed the base branch from plugin-db-infra to main April 4, 2023 21:44
@manasaV3 manasaV3 changed the title Plugin manifest update Update to write Plugin manifest to dynamo and s3 Apr 5, 2023
@manasaV3 manasaV3 changed the base branch from main to plugin-db-infra April 5, 2023 22:01
@manasaV3 manasaV3 requested review from codemonkey800, DragaDoncila, nclack, kne42 and klai95 and removed request for codemonkey800 April 5, 2023 22:02
codemonkey800
codemonkey800 previously approved these changes Apr 6, 2023
View reviewed changes
klai95
klai95 previously approved these changes Apr 7, 2023
View reviewed changes
@manasaV3 manasaV3 mentioned this pull request Apr 10, 2023
Merged
@codemonkey800 codemonkey800 added the new-feature Release Label: Used for categorizing features in automated release notes label Apr 11, 2023
Base automatically changed from plugin-db-infra to main May 8, 2023 23:54
@manasaV3 manasaV3 dismissed stale reviews from klai95 and codemonkey800 via d16d93d May 8, 2023 23:54
@manasaV3 manasaV3 force-pushed the plugin-manifest-update branch 2 times, most recently from 0d48747 to 424a6c0 Compare May 9, 2023 00:09
klai95
klai95 reviewed May 9, 2023
View reviewed changes
plugins/models/plugin.py Outdated Show resolved Hide resolved
klai95
klai95 reviewed May 9, 2023
View reviewed changes
plugins/models/plugin.py Outdated Show resolved Hide resolved
codemonkey800
codemonkey800 previously approved these changes May 16, 2023
View reviewed changes
Copy link
Collaborator

@codemonkey800 codemonkey800 left a comment

Choose a reason for hiding this comment

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

🔥

@manasaV3 @kne42
Format import
99bb518 
Co-authored-by: Kira Evans <contact@kne42.me>
kne42
kne42 reviewed May 17, 2023
View reviewed changes
Copy link
Member

@kne42 kne42 left a comment

Choose a reason for hiding this comment

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

i lack proper context to give this a full review on how the testing/setup is done but overall LGTM. only other thing i might do is assign 'last_updated_timestamp' to a constant in case you ever foresee that parameter's name getting changed but it's really not necessary

@manasaV3 manasaV3 merged commit 9b58fb3 into main May 18, 2023
@manasaV3 manasaV3 deleted the plugin-manifest-update branch May 18, 2023 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kne42 kne42 kne42 left review comments

@klai95 klai95 klai95 left review comments

@richaagarwal richaagarwal richaagarwal approved these changes

@codemonkey800 codemonkey800 codemonkey800 left review comments

@DragaDoncila DragaDoncila Awaiting requested review from DragaDoncila

@nclack nclack Awaiting requested review from nclack

Assignees
No one assigned
Labels
new-feature Release Label: Used for categorizing features in automated release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@manasaV3 @richaagarwal @codemonkey800 @kne42 @klai95