[Refractor] Feature flag #102

SuZhou-Joe · 2023-08-23T10:19:09Z

Description

Issues Resolved

Screenshot

Testing the changes

Check List

codecov-commenter · 2023-08-23T18:16:25Z

Codecov Report

Merging #102 (979015a) into workspace (a058a37) will decrease coverage by 11.03%.
The diff coverage is n/a.

@@              Coverage Diff               @@
##           workspace     #102       +/-   ##
==============================================
- Coverage      65.75%   54.72%   -11.03%     
==============================================
  Files           3336     3038      -298     
  Lines          64493    60368     -4125     
  Branches       10264     9632      -632     
==============================================
- Hits           42406    33038     -9368     
- Misses         19541    25211     +5670     
+ Partials        2546     2119      -427

Flag	Coverage Δ
Linux_1	`34.75% <ø> (ø)`
Linux_3	`42.64% <ø> (-0.01%)`	⬇️
Windows_1	`34.77% <ø> (?)`
Windows_2	`?`
Windows_3	`?`
Windows_4	`34.74% <ø> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 702 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

ruanyl · 2023-08-24T03:35:23Z

src/plugins/workspace/server/routes/index.ts

+      path: `${WORKSPACES_API_BASE_URL}/is_workspace_enabled`,
+      validate: {},
+    },
+    router.handleLegacyErrors(async (context, req, res) => {


Should it call context.core.uiSettings.client.get(FEATURE_FLAG_KEY_IN_UI_SETTING) to get the flag directly from saved objects?

I think that the flag in browser side should be consistent to the flag in server side, but not the opensearch side. The flag in server side may not be consistent to the flag in kibana index because user may call index update API to update the flag directly.

I see, so if changing the flag in the index but not via setWorkspaceFeatureFlag will require a server restart?

Yes, and that is not a expected behavior to update flag.

ruanyl · 2023-08-24T03:42:02Z

src/plugins/workspace/server/plugin.ts

  private config$: Observable<ConfigSchema>;
+  private enabled$: BehaviorSubject<boolean> = new BehaviorSubject(false);
+  private loopRequestTimer?: NodeJS.Timeout;


Was this used anywhere?

Oh, my fault. There was a timer to check the flag in kibana index and try to keep the enabled$ consistent with the real flag in kibana index. Done for removing this.

src/plugins/workspace/server/routes/index.ts

wanglam · 2023-08-24T06:27:11Z

src/plugins/workspace/server/saved_objects/workspace_saved_objects_client_wrapper.ts

@@ -396,7 +396,7 @@ export class WorkspaceSavedObjectsClientWrapper {

    const isDashboardAdmin = this.isDashboardAdmin(wrapperOptions.request);

-    if (isDashboardAdmin) {
+    if (isDashboardAdmin || !this.options.enabled$.getValue()) {


The #scopedSavedObjectsClient in src/core/server/core_route_handler_context.ts won't be re-initialized which means will always use the wrapperOptions.client after enabled$ changed. Is this could be an issue?

Oh, did not know that but it is fine because savedObjectsClient does not rely on enabled$.

I think the client wrapper won't work as expect after enabled change. Such as we can test if the ACL check will be by passed after enabled change.

I see what you mean, when we enable the feature flag, the client is generated with internalRepository, and after we switch the flag, the savedObjectsClient will still be internalSavedObjectsClient, is this what you want to point out?

@wanglam I did some changes to generate savedObjectsClient in runtime, please help to take a look. It should fix the client cache problem.

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

* feat: add some flag Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * refractor: add enabled$ in server side Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add logic check in public side Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feature: remove useless property Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: some modify * feat: merge Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: remove useless code Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: remove useless code Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: optimize code Signed-off-by: SuZhou-Joe <suzhou@amazon.com> --------- Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

This reverts commit e2f277c.

* Revert "[Refractor] Feature flag (#102)" This reverts commit e2f277c.

* feat: add some flag Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * refractor: add enabled$ in server side Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: add logic check in public side Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feature: remove useless property Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: some modify * feat: merge Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: remove useless code Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: remove useless code Signed-off-by: SuZhou-Joe <suzhou@amazon.com> * feat: optimize code Signed-off-by: SuZhou-Joe <suzhou@amazon.com> --------- Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

* Revert "[Refractor] Feature flag (#102)" This reverts commit e2f277c.

SuZhou-Joe marked this pull request as draft August 23, 2023 10:19

github-actions bot added the seasoned-contributor label Aug 23, 2023

SuZhou-Joe added the Skip-Changelog label Aug 24, 2023

SuZhou-Joe marked this pull request as ready for review August 24, 2023 02:55

ruanyl reviewed Aug 24, 2023

View reviewed changes

wanglam reviewed Aug 24, 2023

View reviewed changes

src/plugins/workspace/server/routes/index.ts Outdated Show resolved Hide resolved

SuZhou-Joe force-pushed the feature/feature-flag branch from b27bcb4 to 04814a3 Compare August 24, 2023 08:09

wanglam reviewed Aug 24, 2023

View reviewed changes

SuZhou-Joe force-pushed the feature/feature-flag branch from 91605cc to 53dfc5c Compare August 27, 2023 07:27

SuZhou-Joe added 6 commits August 27, 2023 15:28

feat: add some flag

a7bf202

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

refractor: add enabled$ in server side

28bd816

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

feat: add logic check in public side

47fd0cd

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

feature: remove useless property

7832969

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

feat: some modify

d1e6b90

feat: merge

11ce883

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

SuZhou-Joe force-pushed the feature/feature-flag branch from 4c9e05b to 11ce883 Compare August 27, 2023 07:29

feat: remove useless code

1287116

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

wanglam approved these changes Aug 28, 2023

View reviewed changes

SuZhou-Joe added 2 commits August 28, 2023 11:32

feat: remove useless code

0d92899

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

feat: optimize code

979015a

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>

SuZhou-Joe merged commit e2f277c into ruanyl:workspace Aug 28, 2023
44 of 47 checks passed

ruanyl added a commit that referenced this pull request Sep 5, 2023

Revert "[Refractor] Feature flag (#102)"

d56fab0

This reverts commit e2f277c.

ruanyl added a commit that referenced this pull request Sep 6, 2023

Revert "[Refractor] Feature flag (#102)"

c600ca7

This reverts commit e2f277c.

ruanyl added a commit that referenced this pull request Sep 6, 2023

Revert "[Refractor] Feature flag (#102)" (#124)

58605a8

* Revert "[Refractor] Feature flag (#102)" This reverts commit e2f277c.

SuZhou-Joe pushed a commit that referenced this pull request Sep 6, 2023

Revert "[Refractor] Feature flag (#102)" (#124)

7f1e491

* Revert "[Refractor] Feature flag (#102)" This reverts commit e2f277c.

SuZhou-Joe pushed a commit that referenced this pull request Sep 11, 2023

Revert "[Refractor] Feature flag (#102)" (#124)

55c8efa

* Revert "[Refractor] Feature flag (#102)" This reverts commit e2f277c.

ruanyl added a commit that referenced this pull request Sep 15, 2023

Revert "[Refractor] Feature flag (#102)" (#124)

43b87c1

* Revert "[Refractor] Feature flag (#102)" This reverts commit e2f277c.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Refractor] Feature flag #102

[Refractor] Feature flag #102

SuZhou-Joe commented Aug 23, 2023 •

edited

Loading

codecov-commenter commented Aug 23, 2023 •

edited

Loading

ruanyl Aug 24, 2023

SuZhou-Joe Aug 24, 2023 •

edited

Loading

ruanyl Aug 24, 2023

SuZhou-Joe Aug 24, 2023

ruanyl Aug 24, 2023

SuZhou-Joe Aug 24, 2023

wanglam Aug 24, 2023

SuZhou-Joe Aug 24, 2023

wanglam Aug 24, 2023

SuZhou-Joe Aug 25, 2023

SuZhou-Joe Aug 25, 2023

[Refractor] Feature flag #102

[Refractor] Feature flag #102

Conversation

SuZhou-Joe commented Aug 23, 2023 • edited Loading

Description

Issues Resolved

Screenshot

Testing the changes

Check List

codecov-commenter commented Aug 23, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

SuZhou-Joe Aug 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SuZhou-Joe commented Aug 23, 2023 •

edited

Loading

codecov-commenter commented Aug 23, 2023 •

edited

Loading

SuZhou-Joe Aug 24, 2023 •

edited

Loading