Repository: flyteorg/flyte
Pull request #4280 (2023-10-23 18:45:28)

Rename grpc histogram metrics flag for consistency with flytepropeller
## Tracking issue
https://github.com/flyteorg/flyte/issues/4214


## Describe your changes

See PR title, the recently added admin config option doesn't conform to the existing flytepropeller one

## Check all the applicable boxes <!-- Follow the above conventions to check the box -->

- [ ] I updated the documentation accordingly.
- [ ] All new and existing tests passed.
- [ ] All commits are signed-off.

## Screenshots

<!-- Add all the screenshots which support your changes -->

## Note to reviewers

<!-- Add notes to reviewers if applicable -->


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/flyteorg/flyte/pull/4280?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`5017e6c`)](https://app.codecov.io/gh/flyteorg/flyte/commit/5017e6c34e3495e1b455700e3df3c41077d77858?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) 59.52% compared to head [(`5320d0b`)](https://app.codecov.io/gh/flyteorg/flyte/pull/4280?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) 68.50%.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #4280      +/-   ##
==========================================
+ Coverage   59.52%   68.50%   +8.98%     
==========================================
  Files         632       31     -601     
  Lines       53518     1667   -51851     
==========================================
- Hits        31856     1142   -30714     
+ Misses      19148      447   -18701     
+ Partials     2514       78    -2436     
```

| [Flag](https://app.codecov.io/gh/flyteorg/flyte/pull/4280/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | Coverage Δ | |
|---|---|---|
| [unittests](https://app.codecov.io/gh/flyteorg/flyte/pull/4280/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `68.50% <ø> (+8.98%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg#carryforward-flags-in-the-pull-request-comment) to find out more.

[see 601 files with indirect coverage changes](https://app.codecov.io/gh/flyteorg/flyte/pull/4280/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/flyteorg/flyte/pull/4280?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg).


Patch:
diff --git a/.github/workflows/flyteidl-release.yml b/.github/workflows/flyteidl-release.yml
index ea8a7899f..c69983ce0 100644
--- a/.github/workflows/flyteidl-release.yml
+++ b/.github/workflows/flyteidl-release.yml
@@ -5,7 +5,7 @@ on:
     types: [created]
 
 jobs:
-  deploy:
+  deploy-to-pypi:
     runs-on: ubuntu-latest
     defaults:
       run:
@@ -20,11 +20,11 @@ jobs:
         run: |
           python -m pip install --upgrade pip
           pip install setuptools wheel twine
-      - name: Autobump version
+      - name: Set version in python package
         run: |
           # from refs/tags/v1.2.3 get 1.2.3
           VERSION=$(echo $GITHUB_REF | sed 's#.*/v##')
-          VERSION=$VERSION make -C flyteidl update_pyversion
+          VERSION=$VERSION make update_pyversion
         shell: bash
       - name: Build and publish
         env:
@@ -33,11 +33,17 @@ jobs:
         run: |
           python setup.py sdist bdist_wheel
           twine upload dist/*
+  deploy-to-npm:
+    runs-on: ubuntu-latest
+    defaults:
+      run:
+        working-directory: flyteidl
+    steps:
       - uses: actions/setup-node@v1
         with:
           node-version: "12.x"
           registry-url: "https://registry.npmjs.org"
-      - name: Autobump version
+      - name: Set version in npm package
         run: |
           # from refs/tags/v1.2.3 get 1.2.3
           VERSION=$(echo $GITHUB_REF | sed 's#.*/v##')
diff --git a/flyteadmin/pkg/config/config.go b/flyteadmin/pkg/config/config.go
index 4aafb450e..f6bdd2714 100644
--- a/flyteadmin/pkg/config/config.go
+++ b/flyteadmin/pkg/config/config.go
@@ -47,10 +47,10 @@ type DataProxyUploadConfig struct {
 }
 
 type GrpcConfig struct {
-	Port                 int  `json:"port" pflag:",On which grpc port to serve admin"`
-	ServerReflection     bool `json:"serverReflection" pflag:",Enable GRPC Server Reflection"`
-	MaxMessageSizeBytes  int  `json:"maxMessageSizeBytes" pflag:",The max size in bytes for incoming gRPC messages"`
-	EnableGrpcHistograms bool `json:"enableGrpcHistograms" pflag:",Enable grpc histograms"`
+	Port                     int  `json:"port" pflag:",On which grpc port to serve admin"`
+	ServerReflection         bool `json:"serverReflection" pflag:",Enable GRPC Server Reflection"`
+	MaxMessageSizeBytes      int  `json:"maxMessageSizeBytes" pflag:",The max size in bytes for incoming gRPC messages"`
+	EnableGrpcLatencyMetrics bool `json:"enableGrpcLatencyMetrics" pflag:",Enable grpc latency metrics. Note Histograms metrics can be expensive on Prometheus servers."`
 }
 
 // KubeClientConfig contains the configuration used by flyteadmin to configure its internal Kubernetes Client.
diff --git a/flyteadmin/pkg/config/serverconfig_flags.go b/flyteadmin/pkg/config/serverconfig_flags.go
index 5e6195e63..10229a458 100755
--- a/flyteadmin/pkg/config/serverconfig_flags.go
+++ b/flyteadmin/pkg/config/serverconfig_flags.go
@@ -66,7 +66,7 @@ func (cfg ServerConfig) GetPFlagSet(prefix string) *pflag.FlagSet {
 	cmdFlags.Int(fmt.Sprintf("%v%v", prefix, "grpc.port"), defaultServerConfig.GrpcConfig.Port, "On which grpc port to serve admin")
 	cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "grpc.serverReflection"), defaultServerConfig.GrpcConfig.ServerReflection, "Enable GRPC Server Reflection")
 	cmdFlags.Int(fmt.Sprintf("%v%v", prefix, "grpc.maxMessageSizeBytes"), defaultServerConfig.GrpcConfig.MaxMessageSizeBytes, "The max size in bytes for incoming gRPC messages")
-	cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "grpc.enableGrpcHistograms"), defaultServerConfig.GrpcConfig.EnableGrpcHistograms, "Enable grpc histograms")
+	cmdFlags.Bool(fmt.Sprintf("%v%v", prefix, "grpc.enableGrpcLatencyMetrics"), defaultServerConfig.GrpcConfig.EnableGrpcLatencyMetrics, "Enable grpc latency metrics. Note Histograms metrics can be expensive on Prometheus servers.")
 	cmdFlags.String(fmt.Sprintf("%v%v", prefix, "thirdPartyConfig.flyteClient.clientId"), defaultServerConfig.DeprecatedThirdPartyConfig.FlyteClientConfig.ClientID, "public identifier for the app which handles authorization for a Flyte deployment")
 	cmdFlags.String(fmt.Sprintf("%v%v", prefix, "thirdPartyConfig.flyteClient.redirectUri"), defaultServerConfig.DeprecatedThirdPartyConfig.FlyteClientConfig.RedirectURI, "This is the callback uri registered with the app which handles authorization for a Flyte deployment")
 	cmdFlags.StringSlice(fmt.Sprintf("%v%v", prefix, "thirdPartyConfig.flyteClient.scopes"), defaultServerConfig.DeprecatedThirdPartyConfig.FlyteClientConfig.Scopes, "Recommended scopes for the client to request.")
diff --git a/flyteadmin/pkg/server/service.go b/flyteadmin/pkg/server/service.go
index 4cb5a9036..68ca8856f 100644
--- a/flyteadmin/pkg/server/service.go
+++ b/flyteadmin/pkg/server/service.go
@@ -80,7 +80,7 @@ func newGRPCServer(ctx context.Context, pluginRegistry *plugins.Registry, cfg *c
 	pluginRegistry.RegisterDefault(plugins.PluginIDUnaryServiceMiddleware, grpcmiddleware.ChainUnaryServer(
 		RequestIDInterceptor, auth.BlanketAuthorization, auth.ExecutionUserIdentifierInterceptor))
 
-	if cfg.GrpcConfig.EnableGrpcHistograms {
+	if cfg.GrpcConfig.EnableGrpcLatencyMetrics {
 		logger.Debugf(ctx, "enabling grpc histogram metrics")
 		grpcprometheus.EnableHandlingTimeHistogram()
 	}
diff --git a/flyteidl/setup.py b/flyteidl/setup.py
index 343006932..b82355362 100644
--- a/flyteidl/setup.py
+++ b/flyteidl/setup.py
@@ -6,7 +6,9 @@ setup(
     name='flyteidl',
     version=__version__,
     description='IDL for Flyte Platform',
-    url='https://www.github.com/flyteorg/flyteidl',
+    url='https://github.com/flyteorg/flyte/tree/master/flyteidl',
+    long_description=open("README.md", encoding="utf-8").read(),
+    long_description_content_type="text/markdown",
     maintainer='FlyteOrg',
     maintainer_email='admin@flyte.org',
     packages=find_packages('gen/pb_python'),


Test patch:
diff --git a/flyteadmin/pkg/config/serverconfig_flags_test.go b/flyteadmin/pkg/config/serverconfig_flags_test.go
index 5c991d521..6a95336f4 100755
--- a/flyteadmin/pkg/config/serverconfig_flags_test.go
+++ b/flyteadmin/pkg/config/serverconfig_flags_test.go
@@ -323,14 +323,14 @@ func TestServerConfig_SetFlags(t *testing.T) {
 			}
 		})
 	})
-	t.Run("Test_grpc.enableGrpcHistograms", func(t *testing.T) {
+	t.Run("Test_grpc.enableGrpcLatencyMetrics", func(t *testing.T) {
 
 		t.Run("Override", func(t *testing.T) {
 			testValue := "1"
 
-			cmdFlags.Set("grpc.enableGrpcHistograms", testValue)
-			if vBool, err := cmdFlags.GetBool("grpc.enableGrpcHistograms"); err == nil {
-				testDecodeJson_ServerConfig(t, fmt.Sprintf("%v", vBool), &actual.GrpcConfig.EnableGrpcHistograms)
+			cmdFlags.Set("grpc.enableGrpcLatencyMetrics", testValue)
+			if vBool, err := cmdFlags.GetBool("grpc.enableGrpcLatencyMetrics"); err == nil {
+				testDecodeJson_ServerConfig(t, fmt.Sprintf("%v", vBool), &actual.GrpcConfig.EnableGrpcLatencyMetrics)
 
 			} else {
 				assert.FailNow(t, err.Error())