Migrate es-archiver to typescript #56008

pgayvallet · 2020-01-27T14:37:57Z

Summary

Migrate es-archiver codebase to TS in prevision of potential changes because of #55860

This is a direct / straightforward conversion. The goal was mostly to be able to have proper signatures in src/es_archiver/actions and src/es_archiver/lib

pgayvallet

Some (a lot of) comments and context

pgayvallet · 2020-01-27T15:58:17Z

packages/kbn-test/src/functional_test_runner/lib/config/read_config_file.ts

-export async function readConfigFile(log: ToolingLog, path: string, settingOverrides: any) {
+export async function readConfigFile(log: ToolingLog, path: string, settingOverrides: any = {}) {


Some calls where ignoring the last parameter. Added a default.

pgayvallet · 2020-01-27T15:59:17Z

src/es_archiver/actions/empty_kibana_index.ts

-  await deleteKibanaIndices({ client, stats });
-  await migrateKibanaIndex({ client, log, stats, kibanaPluginIds });
+  await deleteKibanaIndices({ client, stats, log });
+  await migrateKibanaIndex({ client, log, kibanaPluginIds });


Fixed calls with incorrect (additional or missing) parameters

pgayvallet · 2020-01-27T16:00:02Z

src/es_archiver/actions/load.ts

-  const progress = new Progress('load progress');
+  const progress = new Progress();


Progress actually only have a no-args constructor.

pgayvallet · 2020-01-27T16:00:30Z

src/es_archiver/actions/load.ts

-    createCreateIndexStream({ client, stats, skipExisting, log, kibanaPluginIds }),
+    createCreateIndexStream({ client, stats, skipExisting, log }),


Incorrect call parameters, kibanaPluginIds is not used or declared in createCreateIndexStream.

pgayvallet · 2020-01-27T16:02:08Z

src/es_archiver/actions/rebuild_all.ts

    await createPromiseFromStreams([
-      createReadStream(childPath),
+      createReadStream(childPath) as Readable,
      ...createParseArchiveStreams({ gzip }),
      ...createFormatArchiveStreams({ gzip }),
      createWriteStream(tempFile),
-    ]);
+    ] as [Readable, ...Writable[]]);


(There are a lot of these in the PR, will only comment once). Typescript is not able to understand the [Readable, ...Writable[]] signature due to the fact that most transformers are Duplex ( both Readable and Writable). I had to help him.

pgayvallet · 2020-01-27T16:15:24Z

src/es_archiver/lib/indices/delete_index.ts

  const getIndicesToDelete = async () => {
    const aliasInfo = await client.indices.getAlias({ name: index, ignore: [404] });
-    return aliasInfo.status === 404 ? index : Object.keys(aliasInfo);
+    return aliasInfo.status === 404 ? [index] : Object.keys(aliasInfo);
  };

  try {
    const indicesToDelete = await getIndicesToDelete();
    await client.indices.delete({ index: indicesToDelete });
-    stats.deletedIndex(indicesToDelete);
+    for (let i = 0; i < indicesToDelete.length; i++) {
+      const indexToDelete = indicesToDelete[i];
+      stats.deletedIndex(indexToDelete);
+    }


In case of aliases, we were calling stats.deletedIndex with the list of indices, which resulted on incorrect behavior (the call expects a string). Fixed it with a loop.

pgayvallet · 2020-01-27T16:16:33Z

src/es_archiver/lib/indices/kibana_index.ts

-import path from 'path';
+import Path from 'path';


TS error path is already defined in the upper scope for a lot of (path:string) => something handlers. renaming to Path was the easiest solution.

pgayvallet · 2020-01-27T16:17:05Z

src/es_archiver/lib/records/filter_records_stream.ts

-        callback(null, record);
+        callback(undefined, record);


signature expects undefined instead of null

pgayvallet · 2020-01-27T16:18:16Z

src/es_archiver/lib/stats.ts

@@ -37,6 +37,8 @@ export interface IndexStats {
  };
 }

+export type Stats = ReturnType<typeof createStats>;


There is a very strange pattern using inline class definition to access the parent context in this file... Had to use ReturnType to extract a proper TS type.

pgayvallet · 2020-01-27T16:19:12Z

src/legacy/utils/streams/index.d.ts

 export function concatStreamProviders(
-  sourceProviders: Readable[],
+  sourceProviders: Array<() => Readable>,
  options: TransformOptions


Wrong signature definition (as the other changes in this file)

elasticmachine · 2020-01-27T16:20:38Z

Pinging @elastic/kibana-operations (Team:Operations)

elasticmachine · 2020-01-27T16:20:43Z

Pinging @elastic/kibana-platform (Team:Platform)

mistic · 2020-01-29T17:02:12Z

@elasticmachine merge upstream

mistic

for the intent of this PR to be a simple TS transition, it LGTM

mshustov · 2020-01-31T15:00:58Z

ack: will review on Monday

…cript

mshustov · 2020-02-03T07:54:04Z

src/es_archiver/lib/indices/create_index_stream.ts

    if (skipDocsFromIndices.has(record.value.index)) {
      return;
    }

    stream.push(record);
  }

-  async function handleIndex(record) {
+  async function handleIndex(record: any) {


optional: it seems we can define a type for record (probably not full)

interface Record { index: string; type: string; settings: Record<string, any>; // can be imported from SO? mappings: Record<string, any>; aliases: Record<string, any>; }

…cript

kibanamachine · 2020-02-03T10:49:36Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 1828084

History

💚 Build #23933 succeeded 29ceeca
💛 Build #23343 was flaky 32001db
💚 Build #22745 succeeded 48db7b5
💔 Build #22701 failed 5433f71

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* migrate lib/archives and lib/docs * migrate lib/indices * migrate end of /lib * migrate /actions * migrate es_archiver * migrate cli * migrate tests * use proper log stub * add Record typing Co-authored-by: Elastic Machine <[email protected]>

* Migrate es-archiver to typescript (#56008) * migrate lib/archives and lib/docs * migrate lib/indices * migrate end of /lib * migrate /actions * migrate es_archiver * migrate cli * migrate tests * use proper log stub * add Record typing Co-authored-by: Elastic Machine <[email protected]> * fix typings Co-authored-by: Elastic Machine <[email protected]>

pgayvallet added 6 commits January 27, 2020 13:18

migrate lib/archives and lib/docs

a9b3576

migrate lib/indices

c347f1f

migrate end of /lib

a64d369

migrate /actions

efce161

migrate es_archiver

ef2e4c4

migrate cli

5433f71

pgayvallet added v7.7.0 release_note:skip Skip the PR/issue when compiling release notes labels Jan 27, 2020

pgayvallet added 2 commits January 27, 2020 16:57

migrate tests

7c67564

use proper log stub

48db7b5

pgayvallet commented Jan 27, 2020

View reviewed changes

pgayvallet self-assigned this Jan 27, 2020

pgayvallet added the Team:Operations Team label for Operations Team label Jan 27, 2020

pgayvallet added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Jan 27, 2020

pgayvallet marked this pull request as ready for review January 27, 2020 16:20

pgayvallet requested a review from a team as a code owner January 27, 2020 16:20

pgayvallet requested a review from a team January 27, 2020 16:23

Merge branch 'master' into es-archiver-typescript

32001db

mistic approved these changes Jan 29, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into es-archiver-types…

29ceeca

…cript

mshustov approved these changes Feb 3, 2020

View reviewed changes

pgayvallet added 2 commits February 3, 2020 10:25

add Record typing

09c98d4

Merge remote-tracking branch 'upstream/master' into es-archiver-types…

1828084

…cript

pgayvallet merged commit 38dc1cb into elastic:master Feb 3, 2020

pgayvallet mentioned this pull request Feb 3, 2020

[7.x] Migrate es-archiver to typescript (#56008) #56599

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate es-archiver to typescript #56008

Migrate es-archiver to typescript #56008

pgayvallet commented Jan 27, 2020 •

edited

Loading

pgayvallet left a comment •

edited

Loading

pgayvallet Jan 27, 2020

pgayvallet Jan 27, 2020

pgayvallet Jan 27, 2020

pgayvallet Jan 27, 2020

pgayvallet Jan 27, 2020

pgayvallet Jan 27, 2020

pgayvallet Jan 27, 2020

pgayvallet Jan 27, 2020

pgayvallet Jan 27, 2020

pgayvallet Jan 27, 2020

elasticmachine commented Jan 27, 2020

elasticmachine commented Jan 27, 2020

mistic commented Jan 29, 2020

mistic left a comment

mshustov commented Jan 31, 2020

mshustov Feb 3, 2020

kibanamachine commented Feb 3, 2020

		export async function readConfigFile(log: ToolingLog, path: string, settingOverrides: any) {
		export async function readConfigFile(log: ToolingLog, path: string, settingOverrides: any = {}) {

		const progress = new Progress('load progress');
		const progress = new Progress();

		createCreateIndexStream({ client, stats, skipExisting, log, kibanaPluginIds }),
		createCreateIndexStream({ client, stats, skipExisting, log }),

Migrate es-archiver to typescript #56008

Migrate es-archiver to typescript #56008

Conversation

pgayvallet commented Jan 27, 2020 • edited Loading

Summary

pgayvallet left a comment • 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

Choose a reason for hiding this comment

elasticmachine commented Jan 27, 2020

elasticmachine commented Jan 27, 2020

mistic commented Jan 29, 2020

mistic left a comment

Choose a reason for hiding this comment

mshustov commented Jan 31, 2020

Choose a reason for hiding this comment

kibanamachine commented Feb 3, 2020

💚 Build Succeeded

History

pgayvallet commented Jan 27, 2020 •

edited

Loading

pgayvallet left a comment •

edited

Loading