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

Move InvokeGenerator to a centralized function of Meshkit #543

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

dusdjhyeon
Copy link
Member

Description

This PR fixes #541

Notes for Reviewers

  • I moved InvokeGenerator directly into the generators folder, rather than generators/models, due to circular reference issues.
  • The functions I moved to utils can now be made to import meshkit from meshery.
  • The terminal output for registry generate in meshery suggests using io.MultiWriter.

Signed commits

  • Yes, I signed my commits.

@dusdjhyeon dusdjhyeon requested a review from Jougan-0 July 23, 2024 10:31
@Jougan-0
Copy link
Contributor

Mesehryctl needs update as well, right? Can you please update it too and attach that PR with this.
I would recommend to check the working of the generator against Meshery Integration Spreadsheet.

@MUzairS15
Copy link
Contributor

@Jougan-0 @dusdjhyeon Is there any change in logic?

@Jougan-0
Copy link
Contributor

@Jougan-0 @dusdjhyeon Is there any change in logic?

Nope it was just basic migration.
Next on her list is to update the logs on terminal to show the Models that are being generated in real time and if the program is waiting for an Artifacthub rate limit then show the log of Generation paused for x amount of time for Artifacthub registrant.

@Jougan-0
Copy link
Contributor

@dusdjhyeon Did you run this locally? I mean do you have google sheets configured?

@dusdjhyeon
Copy link
Member Author

@dusdjhyeon Did you run this locally? I mean do you have google sheets configured?

I copied meshery Google sheets in my google account and invited my gcp api email and used it.

@Jougan-0
Copy link
Contributor

@dusdjhyeon Did you run this locally? I mean do you have google sheets configured?

I copied meshery Google sheets in my google account and invited my gcp api email and used it.

Was everything working fine and models were generated?

@dusdjhyeon
Copy link
Member Author

@dusdjhyeon Did you run this locally? I mean do you have google sheets configured?

I copied meshery Google sheets in my google account and invited my gcp api email and used it.

Was everything working fine and models were generated?

Yes, I checked.
image

@Jougan-0
Copy link
Contributor

Reviewed on Meshery PR to remove logrus please do so and let's get this merged.

@leecalcote
Copy link
Member

Reviewed on Meshery PR to remove logrus please do so and let's get this merged.

👍

utils/registry/error.go Outdated Show resolved Hide resolved
utils/registry/error.go Outdated Show resolved Hide resolved
utils/registry/error.go Outdated Show resolved Hide resolved
utils/registry/error.go Outdated Show resolved Hide resolved
Copy link

stale bot commented Sep 17, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the issue/stale Issue has not had any activity for an extended period of time label Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/stale Issue has not had any activity for an extended period of time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move InvokeGenerator to a centralized function of Meshkit.
4 participants