feat: set fallback to read local dir for asyncapi file if no context is present

[boyney123]

Description

A fix for #127 it now will check the file directory before finally checking the context again (and throwing errors).

[Souvikns]

This would be a big decision, as the overall functionality would change, I think we need to ask @derberg and @fmvilas for this. Because when we decided on the order of the fallback we agreed on this because the functionality was for the power users who would want to be using the current context if he has spent time setting it up.

[boyney123]

ah OK.

if (filePathOrContextName) {
    const type = await nameType(filePathOrContextName);
    if (type === TYPE_CONTEXT_NAME) {
      return loadFromContext(filePathOrContextName);
    } 
    await fileExists(filePathOrContextName);
    return new SpecificationFile(filePathOrContextName);
  }

Isn't this the part that would check the context? Happens before the fallback?

Or are you talking about the users that setup context, but don't pass in the filepath or context name?

[Souvikns]

That path figures out what that input is. Is the input a file path or a context name this does not run as we are not passing any input as the command is asyncapi start studio

[fmvilas]

Yeah, this same bug hit me during my QCon presentation. I typed asyncapi start studio and it complained there wasn't any context but I had a file in the current working directory. I think this should change.

[derberg]

• there is come console.log in code
• make sure PR title follows imperative mode. Anyway title also needs review to reflect implementation
• context is still important, it is just that in case of no default context, there should not be an error like we get now, but fallback mechanism should work further and in the end the file from root should be picked
• some tests are failing

[boyney123]

Thanks, @derberg.

there is come console.log in code

😅👀 let's pretend you didn't see anything 😂

make sure PR title follows imperative mode. Anyway title also needs review to reflect implementation

Do you mean imperative mood? The first time I heard of this, can you give an example of what you would expect? I changed it, let me know what you think :).

context is still important, it is just that in case of no default context, there should not be an error like we get now, but the fallback mechanism should work further and in the end, the file from root should be picked

OK makes sense. I changed the flow so now will check context, if that fails it will then try and read the local dir for any files. Is this what you mean?

[derberg]

let's pretend you didn't see anything

see what?

Do you mean imperative mood

yes 😅 basically follow https://chris.beams.io/posts/git-commit/#imperative so we stay consistent and we don't get one commit like adding..... and right after add...... Best would be imho feat: set fallback to read local dir for asyncapi file if no context is present. It is of any command and also rather improvement and not fix. What do you think?

OK makes sense. I changed the flow so now will check context, if that fails it will then try and read the local dir for any files. Is this what you mean?

yup, exactly

[boyney123]

chris.beams.io/posts/git-commit/#imperative

Thanks, @derberg this is great and helps loads! I will amend the title 🙇

Best would be imho feat: set fallback to read local dir for asyncapi file if no context is present. It is of any command and also rather improvement and not fix. What do you think?

Makes perfect sense mate, thank you!

[derberg]

LGTM
@Souvikns thoughts?

We still have the problem of course that if no local file is present then ContextError: No context is set as current, please set a current context. is shown. But I think there is a bigger problem with how we are passing errors, as so far I see https://github.com/asyncapi/cli/blob/master/src/errors/validation-error.ts#L19 will never get called even if I do asyncapi validate without context or local file

[Souvikns]

We still have the problem of course that if no local file is present then ContextError: No context is set as current, please set a current context. is shown.

If no file is present in the working directory and also there is no current context set, then ContextError: No context is set as current, please set a current context. will be thrown, also now it is picking the asyncAPI file from the working directory if there is any, thanks to @boyney123 so I think this problem is solved.

But I think there is a bigger problem with how we are passing errors, as so far I see https://github.com/asyncapi/cli/blob/master/src/errors/validation-error.ts#L19 will never get called even if I do asyncapi validate without context or local file

Yeah, this is not something that is supposed to happen. Ok so let me write about the issue in more details:

So when I run asyncapi validate and there is no file in the working directory and no current context is set, previously it threw no-spec-found error which had a message like this -

Unable to perform validation. Specify what AsyncAPI file should be validated.
These are your options to specify in the CLI what AsyncAPI file should be used:
  - You can provide a path to the AsyncAPI file: asyncapi validate path/to/file/asyncapi.yml
  - You can also pass a saved context that points to your AsyncAPI file: asyncapi validate mycontext
  - In case you did not specify a context that you want to use, the CLI checks if there is a default context and uses it. To set default context run: asyncapi context use mycontext
  - In case you did not provide any reference to AsyncAPI file and there is no default context, the CLI detects if in your current working directory you have files like asyncapi.json, asyncapi.yaml, asyncapi.yml. Just rename your file accordingly.

But now this message never gets printed, and instead of this we have a error message like this

ContextError: No context is set as current, please set a current context.

This error is getting thrown here:

cli/src/models/SpecificationFile.ts

Line 43 in b9cc938

return await loadFromContext();

When we do not find any current context we are throwing SpecificationFileNotFound error.

@derberg I think we should open a new issue for this.

[derberg]

@Souvikns yeah, definitely a separate issue, created it here #148

@boyney123 just update with latest master and we get that one merged 🥳

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[boyney123]

Cool thanks @derberg @Souvikns

All updated, good to go 👍

[asyncapi-bot]

🎉 This PR is included in version 0.10.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀