Revert "cli, ui: dismiss release notes signup banner per environment ...variable" #57003
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…variable" This reverts commit dd9613c.
|
This change is |
rytaft
reviewed
Nov 22, 2020
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @knz and @nkodali)
thoszhang
approved these changes
Nov 22, 2020
|
bors r+ |
|
Build succeeded: |
|
Thanks for looking into this and reverting @jordanlewis! Will look into this tomorrow. |
|
No problem! |
This was referenced Nov 24, 2020
srosenberg
added a commit
to srosenberg/cockroach
that referenced
this pull request
Dec 2, 2022
During server startup, reportConfiguration (see cli/start.go), logs the values of CRDB and other, unredacted environment variables to the Ops channel. These environment variables are read from envVarRegistry.cache (see GetEnvVarsUsed) which is assumed to have been populated during the initialization of the (Go) runtime. Specifically, EnvOrDefaultXXX must be invoked, which has a side-effect of populating the cache. When it's invoked outside of the initializer (e.g., inside a func that's not invoked until after initilization), the corresponding environment variable will _not_ be in envVarRegistry.cache. Hence, it will not be reported. Reporting values of modified environment variables is useful for quickly assembling a configuration context during debugging. It's unfortunate that the current design relies on indirectly populating envVarRegistry.cache during the initialization _withoout_ any sort of enforcement. The change in this PR doesn't attempt to fix it. Instead, the following misreported environment variables are now reported, - COCKROACH_SKIP_ENABLING_DIAGNOSTIC_REPORTING - COCKROACH_UPGRADE_TO_DEV_VERSION Note, we also remove COCKROACH_UI_RELEASE_NOTES_SIGNUP_DISMISSED which was only used by roachprod. It has no effect after the PR [1] reverted the related change. Release note: None [1] cockroachdb#57003
srosenberg
added a commit
to srosenberg/cockroach
that referenced
this pull request
Dec 2, 2022
During server startup, reportConfiguration (see cli/start.go), logs the values of CRDB and other, unredacted environment variables to the Ops channel. These environment variables are read from envVarRegistry.cache (see GetEnvVarsUsed) which is assumed to have been populated during the initialization of the (Go) runtime. Specifically, EnvOrDefaultXXX must be invoked, which has a side-effect of populating the cache. When it's invoked outside of the initializer (e.g., inside a func that's not invoked until after initilization), the corresponding environment variable will _not_ be in envVarRegistry.cache. Hence, it will not be reported. Reporting values of modified environment variables is useful for quickly assembling a configuration context during debugging. It's unfortunate that the current design relies on indirectly populating envVarRegistry.cache during the initialization _without_ any sort of enforcement wrt where EnvOrDefaultXXX is invoked. The change in this PR doesn't attempt to fix it. Instead, the following misreported environment variables are now reported, - COCKROACH_SKIP_ENABLING_DIAGNOSTIC_REPORTING - COCKROACH_UPGRADE_TO_DEV_VERSION Note, we also remove COCKROACH_UI_RELEASE_NOTES_SIGNUP_DISMISSED which was only used by roachprod. It has no effect after the PR [1] reverted the related change. Release note: None [1] cockroachdb#57003
craig bot
pushed a commit
that referenced
this pull request
Dec 8, 2022
Remove COCKROACH_UI_RELEASE_NOTES_SIGNUP_DISMISSED which had previously been used by roachprod circa 2020, until it got reverted in [1]. It has no effect after the PR which reverted the related change. Release note: None Epic: None [1] #57003
craig bot
pushed a commit
that referenced
this pull request
Dec 8, 2022
92628: sql: add SHOW TENANT name WITH REPLICATION STATUS r=lidorcarmel a=lidorcarmel Extending SHOW TENANT to also allow showing replication status which contains info such as the protected timestamp on the destination cluster and the source cluster name. Command output right after the destination cluster is created: ``` [email protected]:26257/defaultdb> show tenant dest5 with replication status; id | name | status | source_tenant_name | source_cluster_uri | replication_job_id | replicated_time | retained_time -----+-------+--------+--------------------+--------------------+--------------------+-----------------+---------------- 7 | dest5 | ADD | NULL | NULL | 819890711267737601 | NULL | NULL (1 row) ``` A bit later we have most stats (manually adjusting the source_cluster_uri): ``` [email protected]:26257/defaultdb> show tenant dest5 with replication status; id | name | status | source_tenant_name | source_cluster_uri | replication_job_id | replicated_time | retained_time -----+-------+--------+--------------------+-------------------------------------------------------+--------------------+-----------------+----------------------------- 7 | dest5 | ADD | src | postgresql://[email protected]:26257/defaultdb?ssl...crt | 819890711267737601 | NULL | 2022-12-05 23:00:04.516331 (1 row) ``` And a moment later the replication time is populated. Fixes: #91261 Epic: CRDB-18749 Release note: None 92774: scrun: added a cluster setting to skip planner sanity checks in prod r=Xiang-Gu a=Xiang-Gu Previously, there is a niche case where we might run into a backward incompatible issue (see #91528). We decided to fix it by relaxing the sanity check that caused the issue and backport the change to relase-22.2, so this backward incompatibility issue is contained only between release-22.2.0 and master, which we think should be a rare occurrance in the wild. Fixes: #91528 Release note: None 93103: roachprod: remove COCKROACH_UI_RELEASE_NOTES_SIGNUP_DISMISSED r=nkodali,herkolategan a=srosenberg Remove COCKROACH_UI_RELEASE_NOTES_SIGNUP_DISMISSED which had previously been used by roachprod circa 2020, until it got reverted in [1]. It has no effect after the PR which reverted the related change. Release note: None Epic: None [1] #57003 93150: ui: remove feedback survey link r=Santamaura a=Santamaura This change removes the survey feedback link from the admin ui due to the AppCues survey being retired. Fixes: #92867 Release note (ui change): remove feedback survey link 93215: opt: omit unnecessary assignment casts for UDF return values r=mgartner a=mgartner Previously, UDF return values were being assignment-casted to the return type of the UDF function when the types were identical. These casts are no longer created. Epic: CRDB-20370 Fixes #93210 Release note: None 93255: cli: flag doc updates r=andreimatei a=knz See the individual commits for details. Epic: None Co-authored-by: Lidor Carmel <[email protected]> Co-authored-by: Xiang Gu <[email protected]> Co-authored-by: Stan Rosenberg <[email protected]> Co-authored-by: Santamaura <[email protected]> Co-authored-by: Marcus Gartner <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #57002
This reverts commit dd9613c.
Release note: None