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

Mdpx Conduit Application Changes #878

Open
wants to merge 9 commits into
base: mdpx
Choose a base branch
from
Open

Mdpx Conduit Application Changes #878

wants to merge 9 commits into from

Conversation

anna-cross
Copy link
Contributor

@anna-cross anna-cross commented Mar 21, 2024

Description of change

Changes for conduit application, removing turbine references and usage. Includes new user login as well, we don't need to set env variables, the cli will just prompt the user for inputs.

How to test / use:

  1. Grab the latest mdpx changes and build them locally.
  2. Create a new user with roles appended (I usually just give my user owner role) and verified
  3. Use that user to log in, on first run cli will ask you for tenant url which locally should just be http://localhost:8090
  4. Give your email and password in following prompts
  5. Navigate to mdpx/platform/internal/conduitapps/tests/conduit-project-01
  6. Run meroxa app deploy . --timeout 90s to deploy the app
  7. You should be able to see the app with meroxa app ls
  8. Remove the app by supplying meroxa app remove test-pipeline-2 --force with test-pipeline-2 coming from conduit app name

Fixes https://github.com/meroxa/mdpx/issues/1674

Type of change

  • New feature
  • Bug fix
  • Refactor
  • Documentation

How was this tested?

  • Unit Tests
  • Tested in staging
  • Tested in minikube

Demo

before after

Additional references

Documentation updated

@anna-cross anna-cross changed the base branch from main to mdpx March 21, 2024 22:34
Comment on lines 46 to 47
deploymentCollection = "conduitdeployments"
applicationCollection = "conduitapps"
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

you can wait til having the full deployment functionality in the repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good!

@@ -103,7 +79,7 @@ func (d *Describe) Execute(ctx context.Context) error {
for _, app := range apps.Items {
d.logger.Info(ctx, display.PrintTable(app, displayDetails))
d.logger.JSON(ctx, app)
dashboardURL := fmt.Sprintf("%s/apps/%s/detail", global.GetMeroxaAPIURL(), app.ID)
dashboardURL := fmt.Sprintf("%s/conduitapps/%s/detail", global.GetMeroxaAPIURL(), app.ID)
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -71,7 +71,7 @@ func (l *List) Execute(ctx context.Context) error {
l.logger.Info(ctx, display.PrintList(apps.Items, displayDetails))
l.logger.JSON(ctx, apps)

output := fmt.Sprintf("\n ✨ To view your applications, visit %s/apps", global.GetMeroxaAPIURL())
output := fmt.Sprintf("\n ✨ To view your applications, visit %s/conduitapps", global.GetMeroxaAPIURL())
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -0,0 +1,214 @@
package apps
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no:

  • app.json (one day, we will probably have app.yaml)
  • the need for using git.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm removing this for now until we will add app.yaml support!

@anna-cross anna-cross changed the title Mdpx Conduit Application Mdpx Conduit Application Changes Apr 23, 2024
@anna-cross anna-cross marked this pull request as ready for review April 24, 2024 15:30
lint

lint and tests
DeploymentID []string `json:"deployment_id"`
Name string `json:"name"`
State string `json:"state"`
ApplicationSpec string `json:"stream_tech"`
Copy link
Contributor

Choose a reason for hiding this comment

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

this was renamed to stream_provider not sure where stream_tech is coming from?

@lyuboxa
Copy link
Contributor

lyuboxa commented Apr 26, 2024

@anna-cross can you make another PR which removes all the turbine stuff, then rebase this PR from the other pr, so we can easily review the new code.

@anna-cross anna-cross changed the base branch from mdpx to mdpx_remove_turbine May 14, 2024 16:16
"items": [
{
"collectionId": "77byam8idl1rv8b",
"collectionName": "conduitapps",
Copy link
Contributor

Choose a reason for hiding this comment

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

conduit_apps

Base automatically changed from mdpx_remove_turbine to mdpx May 15, 2024 20:33
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.

3 participants