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

style(cmd-api-server): fix any linter in getOrCreateWebServices() #2751

Merged
merged 1 commit into from
Oct 10, 2023
Merged

style(cmd-api-server): fix any linter in getOrCreateWebServices() #2751

merged 1 commit into from
Oct 10, 2023

Conversation

Yogesh01000100
Copy link
Contributor

Description:

This pull request addresses issue #2676 by fixing an Unexpected any linter warning in the getOrCreateWebServices() method of the ApiServer class. The warning occurred due to a typecast using (app as any).

Changes:

Replaced (app as any) with (app as express.Express)[httpVerbPrometheus as keyof express.Express](httpPathPrometheus, prometheusExporterHandler);

Importance of keyof:

The use of keyof here is crucial because it ensures that httpVerbPrometheus and httpVerb are treated as the valid key of the express.Express type. This is necessary because HTTP verbs, like GET, POST, PUT etc... should be treated as keys within the express.Express type. By casting httpVerbPrometheus as keyof express.Express, we enforce type safety and prevent potential runtime errors that could occur if an invalid HTTP verb is used. In essence, it ensures that only valid HTTP verbs are accepted, improving the robustness of the code.

Benefits:

This change enhances code readability and maintainability using appropriate types and expressions.

Copy link
Contributor

@jagpreetsinghsasan jagpreetsinghsasan left a comment

Choose a reason for hiding this comment

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

Thankyou for the PR, @Yogesh01000100
This looks good to me !

petermetz added a commit to petermetz/cacti that referenced this pull request Oct 9, 2023
…ervices()

This addresses the shortcomings of the linter fix provided in hyperledger-cacti#2751, which
uses unchecked casts to the linter warnings go away.
With the fix of hyperledger-cacti#2751, at runtime, the possibility of a crash is still there
exactly as before, but it has silenced the linter about calling that
possibility out.

We now use a type guard to check the type of the object before casting it
and therefore ensure that at runtime the cast will not produce a crash.

[skip ci]

Depends on hyperledger-cacti#2751
Depends on hyperledger-cacti#2754

Signed-off-by: Peter Somogyvari <[email protected]>
petermetz added a commit to petermetz/cacti that referenced this pull request Oct 9, 2023
… from OAS

This addresses the shortcomings of the linter fix provided in hyperledger-cacti#2751, which
uses unchecked casts to the linter warnings go away.
With the fix of hyperledger-cacti#2751, at runtime, the possibility of a crash is still there
exactly as before, but it has silenced the linter about calling that
possibility out.

We now use a type guard to check the type of the object before casting it
and therefore ensure that at runtime the cast will not produce a crash.

[skip ci]

Depends on hyperledger-cacti#2751
Depends on hyperledger-cacti#2754

Signed-off-by: Peter Somogyvari <[email protected]>
Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@Yogesh01000100 Thank you! This is fine to fix the linter warnings themselves so LGTM (just commenting instead of explicit approval to avoid accidental pre-quorum merge).

For fixing it with runtime type validation baked in, please look at the 2 PRs I just opened We'll merge your first because the linter fixes here are legit and then my 2 PRs coming after it will extend the fix with the runtime type validation. This is just an FYI in case you have the time & willingness to look into the extended solution.

Copy link
Contributor

@izuru0 izuru0 left a comment

Choose a reason for hiding this comment

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

LGTM

@petermetz petermetz merged commit be4bf18 into hyperledger-cacti:main Oct 10, 2023
@Yogesh01000100 Yogesh01000100 deleted the style-lint-fix branch October 14, 2023 04:50
@Yogesh01000100 Yogesh01000100 restored the style-lint-fix branch October 14, 2023 04:50
@Yogesh01000100 Yogesh01000100 deleted the style-lint-fix branch October 17, 2023 18:46
petermetz added a commit to petermetz/cacti that referenced this pull request Oct 17, 2023
… from OAS

This addresses the shortcomings of the linter fix provided in hyperledger-cacti#2751, which
uses unchecked casts to the linter warnings go away.
With the fix of hyperledger-cacti#2751, at runtime, the possibility of a crash is still there
exactly as before, but it has silenced the linter about calling that
possibility out.

We now use a type guard to check the type of the object before casting it
and therefore ensure that at runtime the cast will not produce a crash.

[skip ci]

Depends on hyperledger-cacti#2751
Depends on hyperledger-cacti#2754

Signed-off-by: Peter Somogyvari <[email protected]>
petermetz added a commit to petermetz/cacti that referenced this pull request Oct 18, 2023
… from OAS

This addresses the shortcomings of the linter fix provided in hyperledger-cacti#2751, which
uses unchecked casts to the linter warnings go away.
With the fix of hyperledger-cacti#2751, at runtime, the possibility of a crash is still there
exactly as before, but it has silenced the linter about calling that
possibility out.

We now use a type guard to check the type of the object before casting it
and therefore ensure that at runtime the cast will not produce a crash.

[skip ci]

Depends on hyperledger-cacti#2751
Depends on hyperledger-cacti#2754

Signed-off-by: Peter Somogyvari <[email protected]>
petermetz added a commit that referenced this pull request Oct 18, 2023
… from OAS

This addresses the shortcomings of the linter fix provided in #2751, which
uses unchecked casts to the linter warnings go away.
With the fix of #2751, at runtime, the possibility of a crash is still there
exactly as before, but it has silenced the linter about calling that
possibility out.

We now use a type guard to check the type of the object before casting it
and therefore ensure that at runtime the cast will not produce a crash.

[skip ci]

Depends on #2751
Depends on #2754

Signed-off-by: Peter Somogyvari <[email protected]>
sandeepnRES pushed a commit to sandeepnRES/cacti that referenced this pull request Dec 21, 2023
… from OAS

This addresses the shortcomings of the linter fix provided in hyperledger-cacti#2751, which
uses unchecked casts to the linter warnings go away.
With the fix of hyperledger-cacti#2751, at runtime, the possibility of a crash is still there
exactly as before, but it has silenced the linter about calling that
possibility out.

We now use a type guard to check the type of the object before casting it
and therefore ensure that at runtime the cast will not produce a crash.

[skip ci]

Depends on hyperledger-cacti#2751
Depends on hyperledger-cacti#2754

Signed-off-by: Peter Somogyvari <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants