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

Relocate plugin proto #4826

Merged
merged 2 commits into from Mar 13, 2024
Merged

Relocate plugin proto #4826

merged 2 commits into from Mar 13, 2024

Conversation

khanhtc1202
Copy link
Member

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

  • How are users affected by this change:
  • Is this breaking change:
  • How to migrate (if breaking change):

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
Copy link
Member

@ffjlabo ffjlabo left a comment

Choose a reason for hiding this comment

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

commented 👀

package grpc.pluggin.applicationkind;
option go_package = "github.com/pipe-cd/pipecd/pkg/app/pipedv1/pluggin/applicationkind/api";
package grpc.plugin.platformapi;
option go_package = "github.com/pipe-cd/pipecd/pkg/plugin/platform/platformapi";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
option go_package = "github.com/pipe-cd/pipecd/pkg/plugin/platform/platformapi";
option go_package = "github.com/pipe-cd/pipecd/pkg/plugin/platform/api";

Copy link
Member Author

Choose a reason for hiding this comment

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

we're going to have /stage/api, so I guess we want this package named as platformapi instead of api, or we have to use import aliases each time we import both. wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

@khanhtc1202
I got your point. So how about this? /api/platform and /api/stage like googleapis.
https://github.com/googleapis/googleapis/tree/master/google/api/serviceusage/v1

We can avoid using the import aliases. Also, we can do versioning as /api/platform/v1 when we fix the plugin API.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice point, let's me adopt this 👍

Copy link
Member

Choose a reason for hiding this comment

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

Ah, also we might consider about the plugin versioning like /plugin/v1alpha1/api 👀
WDYT?

Copy link
Member

@ffjlabo ffjlabo Mar 13, 2024

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

📝 talked outside github: We decided to use /plugin/api/v1alpha1/platform to do versioning only the package exposed for the users.

t-kikuc
t-kikuc previously approved these changes Mar 13, 2024
Copy link

codecov bot commented Mar 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.91%. Comparing base (aeb3687) to head (03c3a50).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4826   +/-   ##
=======================================
  Coverage   28.91%   28.91%           
=======================================
  Files         317      317           
  Lines       40333    40333           
=======================================
  Hits        11662    11662           
  Misses      27743    27743           
  Partials      928      928           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
@khanhtc1202
Copy link
Member Author

Updated, please re-approve @ffjlabo @t-kikuc

@khanhtc1202 khanhtc1202 merged commit a98961d into master Mar 13, 2024
21 checks passed
@khanhtc1202 khanhtc1202 deleted the relocate-plugin-proto branch March 13, 2024 04:20
@github-actions github-actions bot mentioned this pull request Apr 8, 2024
t-kikuc pushed a commit that referenced this pull request Apr 8, 2024
Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
t-kikuc pushed a commit that referenced this pull request Apr 8, 2024
Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
t-kikuc added a commit that referenced this pull request Apr 8, 2024
* BUGFIX: Added cancel method to fix context leak (#4767)

Signed-off-by: fazledyn-or <ataf@openrefactory.com>

* Define piped pluggin api (#4815)

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>

* Update BuldPlan API for piped pluggin (#4821)

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>

* Relocate plugin proto (#4826)

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>

* Update controller to use new planner logic (#4825)

* Update controller to use new planner logic

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>

* Update proto path

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>

* Fix typo

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>

* Fix typo

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>

* Update planner logic to call proto instead of self executing

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>

---------

Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>

* Update plugin proto for ExecutorService and add piped pluginservice (#4834)

* Add plugin planner for k8s (#4819)

* [WIP] Add planner

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Not to use out.Version

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Use last_successful_commit_hash and last_successful_config_file_name

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Use in.WorkingDir

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Use in.PipedConfig

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Create git client

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Create secret encryptor

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Add startup server implementation

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Fix for relocation of proto api

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Add roughly implementation for planner plugin

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Rename pkg name

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Add licence

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Comment out for the testing code

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

---------

Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>

* Truncate `deploymentStatus` metrics after reporting stats (#4857)

* Truncate deploymentStatus metrics after reporting to avoid excess message size

Signed-off-by: t-kikuc <tkikuchi07f@gmail.com>

* Rename func to Flush() for clarity

Signed-off-by: t-kikuc <tkikuchi07f@gmail.com>

* Add comment of what's included in statsreporter's body

Signed-off-by: t-kikuc <tkikuchi07f@gmail.com>

* Fix indent in the comment

Signed-off-by: t-kikuc <tkikuchi07f@gmail.com>

* Copy change of metrics.go to pipedv1

Signed-off-by: t-kikuc <tkikuchi07f@gmail.com>

* Copy change of reporter.go to pipedv1

Signed-off-by: t-kikuc <tkikuchi07f@gmail.com>

---------

Signed-off-by: t-kikuc <tkikuchi07f@gmail.com>

---------

Signed-off-by: fazledyn-or <ataf@openrefactory.com>
Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
Signed-off-by: Yoshiki Fujikane <ffjlabo@gmail.com>
Signed-off-by: t-kikuc <tkikuchi07f@gmail.com>
Co-authored-by: Ataf Fazledin Ahamed <ataf@openrefactory.com>
Co-authored-by: Khanh Tran <32532742+khanhtc1202@users.noreply.github.com>
Co-authored-by: Yoshiki Fujikane <40124947+ffjlabo@users.noreply.github.com>
@t-kikuc t-kikuc mentioned this pull request Apr 9, 2024
sZma5a pushed a commit to sZma5a/pipecd that referenced this pull request Apr 18, 2024
Signed-off-by: khanhtc1202 <khanhtc1202@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants