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

fix: add --version flag #620

Merged
merged 7 commits into from
Jul 28, 2021
Merged

fix: add --version flag #620

merged 7 commits into from
Jul 28, 2021

Conversation

TarikGul
Copy link
Member

@TarikGul TarikGul commented Jul 24, 2021

closes: #484

This adds the version flag for sidecar, but currently there is some unknown default behavior that should be resolved before this is merged.

Run: yarn build && yarn start --version

Current output:

SAS:
  📦 LOG:
     ✅ LEVEL: "info"
     ✅ JSON: false
     ✅ FILTER_RPC: false
     ✅ STRIP_ANSI: false
  📦 SUBSTRATE:
     ✅ WS_URL: "wss://rpc.polkadot.io"
     ✅ TYPES_BUNDLE: undefined
     ✅ TYPES_CHAIN: undefined
     ✅ TYPES_SPEC: undefined
     ✅ TYPES: undefined
  📦 EXPRESS:
     ✅ BIND_HOST: "127.0.0.1"
     ✅ PORT: 8080
@substrate/api-sidecar v9.0.0

I am under the impression that this could be the root of the issue, but I need to do some more digging.

It originated from this import, then trickles down to ./controllers all the way to anywhere validateAddress is imported in, such as all the Account Controllers.

Update:

This is fixed, and working as it should!

@TarikGul TarikGul requested a review from emostov July 25, 2021 13:10
src/main.ts Outdated Show resolved Hide resolved
src/Cli.ts Outdated Show resolved Hide resolved
@emostov
Copy link
Contributor

emostov commented Jul 26, 2021

but currently there is some unknown default behavior that should be resolved before this is merged.

Whats the exact behavior that is unexpected?

@TarikGul
Copy link
Member Author

@emostov The "unknown" issue is just SAS is logging/printing the below by just importing files. As in without initializing anything this will always print if main.ts is called.

SAS:
  📦 LOG:
     ✅ LEVEL: "info"
     ✅ JSON: false
     ✅ FILTER_RPC: false
     ✅ STRIP_ANSI: false
  📦 SUBSTRATE:
     ✅ WS_URL: "wss://rpc.polkadot.io"
     ✅ TYPES_BUNDLE: undefined
     ✅ TYPES_CHAIN: undefined
     ✅ TYPES_SPEC: undefined
     ✅ TYPES: undefined
  📦 EXPRESS:
     ✅ BIND_HOST: "127.0.0.1"
     ✅ PORT: 8080

@emostov
Copy link
Contributor

emostov commented Jul 26, 2021

@TarikGul try moving this line inside of main:

const { config } = SidecarConfig;

I think its calling the getter here:

static get config(): ISidecarConfig {
return this._config || this.create();
}

@TarikGul
Copy link
Member Author

TarikGul commented Jul 26, 2021

@TarikGul try moving this line inside of main:

const { config } = SidecarConfig;

I think its calling the getter here:

static get config(): ISidecarConfig {
return this._config || this.create();
}

Yea I had tried this earlier when trying to debug it and it didn't work, that could be another source of where it could be coming from though, but I also can confirm that in this file specifically this line it's being called during the imports.

But that being said it I think it could be initializing in more than one place too.

@TarikGul
Copy link
Member Author

@emostov I did make some progress on it, looks like you were right in one respect. consoleTransport had the same issue with SidecarConfig which you pointed out in main. So three cases where it shows up is now solved (main.ts, consoleTransport.ts). The issue is still coming up, but I think I should have it solved shortly after having those 3 sorted out.

Copy link
Contributor

@emostov emostov left a comment

Choose a reason for hiding this comment

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

💯

src/main.ts Show resolved Hide resolved
Copy link
Collaborator

@jsdw jsdw left a comment

Choose a reason for hiding this comment

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

Code looks good to me, and it does what it says on the tin :)

@TarikGul TarikGul merged commit 9d8bb98 into master Jul 28, 2021
@TarikGul TarikGul deleted the tarik-cli-version branch July 28, 2021 18:04
@jsdw jsdw mentioned this pull request Aug 2, 2021
This pull request was closed.
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.

Add a --version flag
3 participants