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

Detect file type #1935

Merged

Conversation

sudeeptarlekar
Copy link
Collaborator

Rustcore should have functionality to detect the file type. If file type is binary or text, and if text what is the encoding of file.

Resolves: #1841

@sudeeptarlekar sudeeptarlekar force-pushed the 1841-file-type-detection branch 2 times, most recently from d5a9cca to 9899a74 Compare October 19, 2023 15:03
@DmitryAstafyev
Copy link
Collaborator

DmitryAstafyev commented Oct 20, 2023

Location

We have a nice suitable place for it application/apps/indexer/addon, but now it's just one module. I would suggest (if @marcmo agree):

  • rename application/apps/indexer/addon into application/apps/indexer/addons
  • current lib in application/apps/indexer/addon rename into dlt-tools and add as submodule
  • same for your tool, create files-tools and add there

It will be something like:

-- addons
---- dlt-tools
------ src
------ Cargo.tolm
---- files-tools
------ src
------ Cargo.tolm

Create it as lib and add tests of course.

Artifacts for tests

Also, take into account, we have this place developing/resources. Here you can store resources for testing. You can left there some example files, which you would use for testing.

General idea

I think reading some parts of target-file and searching "specific binary or not binary" symbols is a good idea and the right direction. Just a question, would it be simple just try some chunks of file with std::str::from_utf8. Because if it isn't UTF8: you will get error or result string len will be != to the len of given byte's slice. That's just an idea, I don't know what is a better.

Good habit

As soon as it's draft, no problems of course to skip cargo clippy, but I would recommend doing it always before pushing with rake clippy:all... As usual, it gives you nice and useful recommendations about code styling and allows you to better understand rust philosophy

@DmitryAstafyev
Copy link
Collaborator

DmitryAstafyev commented Oct 20, 2023

DO NOT MERGE in the name of PR scares me )) sounds like a bomb in our solution)) If your PR in the draft state it will not be merged - no worries )

@DmitryAstafyev
Copy link
Collaborator

linked to: #1921, #1841

@DmitryAstafyev DmitryAstafyev added native implemented in the rust part (natively) rust Pull requests that update Rust code labels Oct 20, 2023
@marcmo
Copy link
Member

marcmo commented Oct 20, 2023

indexer/addon is a good place...no need to rename to addons

@sudeeptarlekar sudeeptarlekar force-pushed the 1841-file-type-detection branch 2 times, most recently from ac37937 to ce778fb Compare October 25, 2023 13:08
@sudeeptarlekar sudeeptarlekar changed the title [DO NOT MERGE] Detect file type Detect file type Oct 25, 2023
@sudeeptarlekar sudeeptarlekar force-pushed the 1841-file-type-detection branch 3 times, most recently from 54a6b2c to acf3bfe Compare October 26, 2023 13:20
@sudeeptarlekar sudeeptarlekar force-pushed the 1841-file-type-detection branch 3 times, most recently from e8e32d3 to 681dc72 Compare November 13, 2023 10:18
@sudeeptarlekar sudeeptarlekar marked this pull request as ready for review November 13, 2023 10:23
@sudeeptarlekar sudeeptarlekar force-pushed the 1841-file-type-detection branch 2 times, most recently from 86eb51b to 6bbdf21 Compare November 16, 2023 11:37
@sudeeptarlekar
Copy link
Collaborator Author

Is this good to 🚢 ? Asking as I was bit skeptical about changes...

@sudeeptarlekar sudeeptarlekar force-pushed the 1841-file-type-detection branch from 6bbdf21 to 59df6db Compare January 8, 2024 15:10
@sudeeptarlekar sudeeptarlekar force-pushed the 1841-file-type-detection branch 6 times, most recently from da05549 to ad60f99 Compare January 16, 2024 10:44
@sudeeptarlekar sudeeptarlekar force-pushed the 1841-file-type-detection branch 3 times, most recently from 986cc9a to 5dc942a Compare February 9, 2024 13:26
application/holder/src/service/bridge/file/file.ts Outdated Show resolved Hide resolved
application/holder/src/service/bridge/folder/select.ts Outdated Show resolved Hide resolved
application/holder/src/service/bridge/folder/select.ts Outdated Show resolved Hide resolved
application/holder/src/service/bridge/folder/select.ts Outdated Show resolved Hide resolved
application/holder/src/service/bridge/folder/select.ts Outdated Show resolved Hide resolved
@@ -34,7 +34,7 @@ export class Action extends CLIAction {
return this.error;
}

public execute(cli: Service): Promise<void> {
public async execute(cli: Service): Promise<void> {
Copy link
Collaborator

@DmitryAstafyev DmitryAstafyev Feb 13, 2024

Choose a reason for hiding this comment

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

I don't remember (please check it), but if fucntion is async, we can return just Error or throw new Error(), instead Promise.reject(Error)... but please check it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here is a good example to better understand promises and async stuff.

async function a() {
    return new Error('Test');
}

async function b() {
    return Promise.reject(new Error('Test'));
}

async function c() {
    throw new Error('Test');
}

async function d() {
    return new Promise(() => {
        throw new Error('Test');
    })
}

a().then((_) => {
    console.error(`[A]: CATCH: isn't handled`);
}).catch((_) => {
    console.log('[A] - ok');
});

b().then((_) => {
    console.error(`[B]: CATCH: isn't handled`);
}).catch((_) => {
    console.log('[B] - ok');
});

c().then((_) => {
    console.error(`[C]: CATCH: isn't handled`);
}).catch((_) => {
    console.log('[C] - ok');
});

d().then((_) => {
    console.error(`[D]: CATCH: isn't handled`);
}).catch((_) => {
    console.log('[D] - ok');
});

console:

[A]: CATCH: isn't handled
[C] - ok
[B] - ok
[D] - ok

Well returning new Error will not reject promise, but return result with resolve

@sudeeptarlekar sudeeptarlekar force-pushed the 1841-file-type-detection branch 2 times, most recently from df97940 to a760264 Compare February 14, 2024 14:12
Rustcore should have functionality to detect the file type. If
file type is binary or text, and if text what is the encoding of
file.

Resolves: esrlabs#1841
Added function in rs bindings to that it will be available in the
node bindings on frontend.
Added code in TS to use the file detection functionality from
rust core.
@sudeeptarlekar sudeeptarlekar force-pushed the 1841-file-type-detection branch from a760264 to 78858ba Compare February 19, 2024 12:10
application/apps/indexer/addons/file-tools/src/lib.rs Outdated Show resolved Hide resolved
application/apps/indexer/addons/file-tools/src/lib.rs Outdated Show resolved Hide resolved
application/holder/src/env/fs/index.ts Outdated Show resolved Hide resolved
application/holder/src/env/fs/index.ts Outdated Show resolved Hide resolved
Comment on lines 100 to 101
try {
const stat = fs.statSync(filename);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please make sure statements try/catch are still actual after switching to async code

@@ -34,7 +34,7 @@ export class Action extends CLIAction {
return this.error;
}

public execute(cli: Service): Promise<void> {
public async execute(cli: Service): Promise<void> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here is a good example to better understand promises and async stuff.

async function a() {
    return new Error('Test');
}

async function b() {
    return Promise.reject(new Error('Test'));
}

async function c() {
    throw new Error('Test');
}

async function d() {
    return new Promise(() => {
        throw new Error('Test');
    })
}

a().then((_) => {
    console.error(`[A]: CATCH: isn't handled`);
}).catch((_) => {
    console.log('[A] - ok');
});

b().then((_) => {
    console.error(`[B]: CATCH: isn't handled`);
}).catch((_) => {
    console.log('[B] - ok');
});

c().then((_) => {
    console.error(`[C]: CATCH: isn't handled`);
}).catch((_) => {
    console.log('[C] - ok');
});

d().then((_) => {
    console.error(`[D]: CATCH: isn't handled`);
}).catch((_) => {
    console.log('[D] - ok');
});

console:

[A]: CATCH: isn't handled
[C] - ok
[B] - ok
[D] - ok

Well returning new Error will not reject promise, but return result with resolve

@@ -34,25 +34,23 @@ export class Action extends CLIAction {
return this.error;
}

public execute(cli: Service): Promise<void> {
public async execute(cli: Service): Promise<void> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please also make sure, all places of this function's usage are checked and corrected also. It's quite radical change (sync -> async) and we should make sure - all calls of changed functions are corrected too.

@sudeeptarlekar sudeeptarlekar force-pushed the 1841-file-type-detection branch 2 times, most recently from 9fba51c to 2d62218 Compare February 21, 2024 11:46
@sudeeptarlekar sudeeptarlekar force-pushed the 1841-file-type-detection branch from 2d62218 to b26d568 Compare February 27, 2024 14:06
We converted execute method to asynchronous from synchronous
and this commit wraps the usage of that async method inside try
block to avoid errors
@DmitryAstafyev DmitryAstafyev merged commit 23da4e8 into esrlabs:master Mar 1, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
native implemented in the rust part (natively) rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

File type recognize
3 participants