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

Make contexts shareable #38

Closed
fmvilas opened this issue Jul 26, 2021 · 37 comments · Fixed by #567
Closed

Make contexts shareable #38

fmvilas opened this issue Jul 26, 2021 · 37 comments · Fixed by #567
Assignees
Labels
bounty AsyncAPI Bounty program related label enhancement New feature or request level/advanced AsyncAPI Bounty program related label

Comments

@fmvilas
Copy link
Member

fmvilas commented Jul 26, 2021

Reason/Context

Currently, contexts are stored in the user's home directory. I think it would be cool to store the contexts in the repo directory so they can be shared.

Description

This would work the same way package.json works. The asyncapi CLI would seek for the file (let's call it .asyncapi) in the CWD and, if not found, it would look for it in the parent directory, then the parent of the parent, etc.

The cool thing is that if you don't want to share it, you could just place the file in your home dir, effectively behaving as it is now (global). Also, the global context would be the fallback if a "local" one is not found in a specific project.

Also, this would remove the need to set up contexts yourself manually. Since the file is already in the repo, running asyncapi will already load the context automatically.

Additionally, this eliminates the problem where a user doesn't have permission to write to disk. Very likely to happen in private built in-house CI systems (this was the case at New Relic).

@fmvilas fmvilas added the enhancement New feature or request label Jul 26, 2021
@derberg
Copy link
Member

derberg commented Jul 26, 2021

I like this idea a lot!

@Souvikns
Copy link
Member

Souvikns commented Sep 3, 2021

would like to start by talking about the data structure for context. All context is doing right now is storing file paths that supposedly point to a spec path. Storing it in a key-value pair. So context in general looks something like this.

interface Context {
  current?: string,
  store: {
    [name: string]: string
  }
}

ContextService is loading this data and providing functions to read and manipulate this. Now the problem is there is a case that the global .asyncapi might be missing. For this case, the loader is throwing a ContextFileNotFoundError which serves the purpose, but this means we have to check for this error every time we are using this. This is making the code messy, I would like to give some examples -

While autoloading the spec file, there was a case where when there was no global .asyncapi file it was unable to autodetect spec files in the working directory as it was throwing the ContextFileNotFoundError. Had to work around like this.

} catch (error) {
if (error instanceof ContextFileNotFoundError) {
try {
specFile = autoDetectSpecFileInWorkingDir(contextService.autoDetectSpecFile(), cliService.command());
return { specFile };
} catch (error) {
return { error };
}
}
return { error };
}

Was playing around with the idea that we don't throw an error rather return an object with undefined context.

Something like this.
import * as fs from 'fs';
import * as path from 'path';


const SPEC_FILE_PATH = path.resolve(__dirname, 'asyncapi');

interface Context {
  current?: string,
  store: {
    [name: string]: string
  }
}

const saveContext = (ctx: Context) => {
  fs.writeFileSync(SPEC_FILE_PATH, JSON.stringify(ctx), {encoding: 'utf-8'});
}

class ContextService{
  constructor(private _context:Context | undefined){
    
  }

  current(){
    return this._context?.current;
  }

  context(contextName?:string){
    if(!contextName) return this._context;
    return this._context?.store[contextName];
  }

  isContext(){
    return typeof this._context !== 'undefined'
  }

  addContext(name: string, specPath: string){
    if(this._context) {
      this._context.store[name] = specPath;
      saveContext(this._context)
    }
    return true
  }

  static load(){
    try{
      const ctx = fs.readFileSync(SPEC_FILE_PATH, 'utf-8');
      return new ContextService(JSON.parse(ctx) as Context);
    }catch(e) {
      return new ContextService(undefined);
    }
  }
}

const ctxService = ContextService.load();
console.log(ctxService.isContext());
console.log(ctxService.addContext('test', 'url/to/specFile'))

@fmvilas
Copy link
Member Author

fmvilas commented Sep 6, 2021

Yeah, I don't think it should throw an error as it's optional to have .asyncapi files. Much better to return undefined IMHO.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2022

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Jan 5, 2022
@fmvilas fmvilas removed the stale label Jan 7, 2022
@fmvilas
Copy link
Member Author

fmvilas commented Jan 7, 2022

Removing stale label as we still plan to implement this. If someone is up for this, it's a good one!

@github-actions
Copy link
Contributor

github-actions bot commented May 8, 2022

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label May 8, 2022
@derberg derberg removed the stale label May 10, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2022

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added stale and removed stale labels Sep 8, 2022
@fmvilas
Copy link
Member Author

fmvilas commented Sep 9, 2022

Dear bot, keep this issue open. Thanks!

@github-actions
Copy link
Contributor

github-actions bot commented Jan 8, 2023

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Jan 8, 2023
@fmvilas
Copy link
Member Author

fmvilas commented Jan 10, 2023

Still relevant.

@github-actions github-actions bot removed the stale label Jan 11, 2023
@derberg
Copy link
Member

derberg commented Apr 17, 2023

The scope here is to:

  • make CLI read context configuration from CWD
  • make current context home directory storage a fallback to new CWD based
  • prepare code for an option to long-term have a flag in every command to specify a custom location of the context. So instead of home or CWD, someone can specify custom file system location of context config. Do not implement, outcome should be a proper amount of good-first-issues that enable further implementation

@derberg derberg added bounty AsyncAPI Bounty program related label level/advanced AsyncAPI Bounty program related label labels Apr 19, 2023
@thulieblack
Copy link
Member

This issue is part of the AsyncAPI Bounty Program, interested contributors need to follow the guidelines below to ensure fairness and timely completion of assigned tasks:

Task Assignment

The assignment of tasks will be prioritized in the following order:

  • AsyncAPI maintainers (from any repository)
  • Regular contributors (individuals who have merged three or more pull requests)
  • Other (if an individual doesn't fall under the above, the maintainer can determine the criteria i.e regular volunteers, etc)

We encouraged everyone to apply as long as the task is for you (falls under your skill set). We will not be using the first comment - get assigned method for assignments. Instead, we will provide 3 days to consider all individuals interested before assigning any bounty task.

Deadline

To maintain accountability and ensure the timely completion of the deadline for this task will be 6 - 8 weeks from the date of assignment. If a contributor requires an extension on their task, it should be communicated and approved by the maintainer.

Issue Tracking and Updates

Contributors must provide weekly updates on the task they are working on to help maintainers stay informed of progress. If a contributor fails to provide an update, they will be reminded via a ping. If a contributor fails to provide an update after three pings over three weeks, we will assume they have silently dropped the issue, and it will be reassigned to someone else.

Issue Drop-outs

Any contributor who drops an assigned issue will be penalized and will not be eligible to participate in the bounty program for 6 months. We understand that unforeseen circumstances can arise, and dropping an assigned issue may be unavoidable in some cases. However, we believe that enforcing this penalty is necessary to ensure the effectiveness of the bounty program, respect maintainer time, and honor the efforts of other contributors who could have solved the issue but were unable to do so due to the drop-out.

We encourage all contributors to follow these guidelines to ensure a fair and timely completion of tasks assigned through our bounty program.

@derberg
Copy link
Member

derberg commented Apr 25, 2023

more context for the issue:

for now context are a “global” thing, like bash profile or git config -> os.homedir()

the idea is that this approach should not be default, but a fallback

by default, CLI should search for .asyncapi (probably we should rename to .asyncapi_cli 🤔 ) in the CWD and then cascading search for it up to the root of the repository - then if not found, look for the global one

so for example you have context file only for given repository, keep it in git with rest of the code, or have few context files in one repo in case you have mono repo

@aeworxet
Copy link
Contributor

I would like to work on this issue.

@derberg
Copy link
Member

derberg commented Apr 26, 2023

@aeworxet perfect, please go ahead and ask questions whenever help is needed.

@aeworxet
Copy link
Contributor

aeworxet commented May 1, 2023

Some questions.

1

What should happen in this situation?

/home/userhomedir/web_development/personal/foss/asyncapi/cli/fork/src/models/.asyncapi
/home/userhomedir/web_development/personal/foss/asyncapi/cli/fork/src/.asyncapi
/home/userhomedir/web_development/personal/foss/asyncapi/cli/fork/.asyncapi
/home/userhomedir/.asyncapi

Should the search stop on first found .asyncapi file?

2

Is there a real .asyncapi file with real data so I could test not only its existence or whether it can be simply read, but whether it can be IMPORTED and whether it satisfies all requirements of interface IContextFile {}?

@aeworxet
Copy link
Contributor

aeworxet commented May 5, 2023

image

@derberg
Copy link
Member

derberg commented May 9, 2023

1

if you run asyncapi in /home/userhomedir/web_development/personal/foss/asyncapi/cli/fork/src/models it should pick /home/userhomedir/web_development/personal/foss/asyncapi/cli/fork/src/models/.asyncapi

not sure it answers your question? cwd basically

2

you can create it yourself, just do asyncapi context add for example

and here is an example with complex project structure and multiple documents https://github.com/asyncapi/spec/tree/master/examples/social-media

@thulieblack
Copy link
Member

Hey @aeworxet how is it going so far?

@aeworxet
Copy link
Contributor

Working closely with @derberg to ensure the development is going in right direction.
Thought last week I'm ready for a draft PR but no.

@aeworxet
Copy link
Contributor

Bounty Program Timeline

Category Assigned on (by GitHub) Started on (by BP rules) Due (by BP rules) Draft PR submission Final PR submission Final PR merge
Advanced Level 2023-04-26 2023-01-05 2023-06-23 (Friday of eighth week) 2023-05-19 (Friday of third week and updated on every Friday) 2023-06-09 (Friday of sixth week) 2023-06-23 (Friday of eighth week)

Please note that dates represent deadlines, not specific dates, so if the goal is reached sooner, it's better.

@aeworxet
Copy link
Contributor

aeworxet commented Jun 1, 2023

Should CLI context file's name be changed from .asyncapi to .asyncapi-cli, by analogy with .asyncapi-tool?

Copy link
Member

derberg commented Jun 1, 2023

no strong opinion from my side, both are ok I think, I do not see scenario where in one repo someone would have .asyncapi-tool and .asyncapi. So .asyncapi is pretty ok I think

@aeworxet
Copy link
Contributor

aeworxet commented Jun 1, 2023

If there's no strong opinion, I'd rather make it .asyncapi-cli. Just in case there appears a Big Universal AsyncAPI Framework at some moment which will proudly utilize filename .asyncapi.
And while this context functionality is not much in use, because the more users use it, the harder it is to migrate to a new name in case of a need.

Copy link
Member

derberg commented Jun 1, 2023

Big Universal AsyncAPI Framework at some moment which will proudly utilize filename .asyncapi

I like that! 😃

@aeworxet
Copy link
Contributor

aeworxet commented Jun 6, 2023

image

PR update postponed until Executive Director returns from vacation.

@thulieblack
Copy link
Member

A break for you 😊

@derberg
Copy link
Member

derberg commented Jun 19, 2023

Executive Director 👀

next time please do not feel blocked by me, there is also @Souvikns that maintains CLI and also the request for shared context came from @fmvilas

but I also understand that you preferred to wait as we discusses some technical details before my holidays.

PR is in Draft mode, so please lemme know if ready for review

@aeworxet
Copy link
Contributor

This was at the same time a Bounty Program usecase. If your vacation would INDEED be a blocking issue, how would the Bounty Program timeline change?
I would propose to increase any stage the issue is at that moment in, by duration of absence of the person in charge from the AsyncAPI side + 1 week, because if such person (Core Developer?) was absent for a month or two (for any reason), the bounty hunter would then have to spend some time getting back to the insides of the issue and nearly unfamiliar at that time his own code.

@aeworxet
Copy link
Contributor

Executive Director

As a giggly alternative I also have Chief Merging Officer.

@derberg
Copy link
Member

derberg commented Jun 21, 2023

As a giggly alternative I also have Chief Merging Officer.

ok, but only if we write it Chief mErging Officer so I can say I'm a CEO 😆

If your vacation would INDEED be a blocking issue, how would the Bounty Program timeline change?

if I'm a blocker and on holidays or basically maintainer is off and do not respond for some time -> for me it is clear that the time is then like on hold, counter stops

@aeworxet
Copy link
Contributor

It's logical that the time of actual absence is added to the timeline. It is +1 week that needs clarification.

@derberg
Copy link
Member

derberg commented Jun 21, 2023

sounds good to me, makes sense to add +1. Please add it as feedback comment to the related bounty discussion so we do not forget to address it when we make official bounty program. Let's further discuss there and not in this issue

@aeworxet
Copy link
Contributor

Bounty Program Timeline

(target dates are updated by the duration of absence of the person in charge from AsyncAPI side in the amount of two weeks + one week)

Category Assigned on (by GitHub) Started on (by BP rules) Due (by BP rules) Draft PR submission Final PR submission Final PR merge
- - - 2023-07-14 (increased by three weeks) - 2023-06-30 (increased by three weeks) 2023-07-14 (increased by three weeks)
Advanced Level 2023-04-26 2023-01-05 2023-06-23 (Friday of eighth week) 2023-05-19 (Friday of third week and updated on every Friday) 2023-06-09 (Friday of sixth week) 2023-06-23 (Friday of eighth week)

@derberg
Copy link
Member

derberg commented Jul 12, 2023

@thulieblack fyi it is taking a bit longer because @aeworxet is adding one more small docs to it and also documentation for the context concept that we did not have initially in the scope

derberg added a commit to aeworxet/asyncapi-cli that referenced this issue Aug 7, 2023
@derberg
Copy link
Member

derberg commented Aug 7, 2023

@thulieblack last bounty issue completed 🥳

@aeworxet congrats 👏🏼

@thulieblack
Copy link
Member

Congratulations @aeworxet 🎉🎉

Please submit an expense via open collective to claim` your bounty

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty AsyncAPI Bounty program related label enhancement New feature or request level/advanced AsyncAPI Bounty program related label
Projects
Status: Completed
Development

Successfully merging a pull request may close this issue.

5 participants