Skip to content

Conversation

@janelletavares
Copy link
Contributor

@janelletavares janelletavares commented May 26, 2023

Description of change

Don't let the papi db get cluttered without a way to clean it up.

Fixes https://github.com/meroxa/turbine-project/issues/291

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

@justmisosoup
Copy link
Contributor

justmisosoup commented May 30, 2023

@janelletavares How difficult possible would it be to rework the meroxa flink command to meroxa jobs?

To clarify, not a necessary change as this will likely change in the future when we might eventually incorporate into apps, but Raùl had raised a good point about consistency of what we use in the CLI. E.g., apps being similar to jobs vs using the framework.

@ericcheatham
Copy link
Contributor

@justmisosoup It's a fairly low effort change overall. Though that does raise a question about how we should refer to jobs in documentation/help text. Instead of

Remove flink job

should it be something like

Remove job

that feels non-specific.

cc @janelletavares

@justmisosoup
Copy link
Contributor

justmisosoup commented May 30, 2023

@ericcheatham Good point. Articulating Remove Flink job. in the CLI documentation would be consistent with what we do with the apps commands.

Today, that looks like in the Meroxa CLI:

Available Commands:
  deploy      Deploy a Turbine Data Application
  describe    Describe a Turbine Data Application
  init        Initialize a Turbine Data Application
  list        List Turbine Data Applications
  logs        View relevant logs to the state of the given Turbine Data Application

And in the documentation itself: https://docs.meroxa.com/cli/cmd/meroxa-apps#see-also

Looking forward to the future for when Flink is eventually encapsulated under apps we'll likely need to scrub Turbine Data Application from the CLI documentation and come up with something more generalised.

cc: @janelletavares

@janelletavares
Copy link
Contributor Author

@justmisosoup I believe it's a one-liner to change meroxa flink to meroxa jobs. There are no generated docs for these commands yet because they are hidden, so nothing to update around there.

Copy link
Contributor

@jayjayjpg jayjayjpg left a comment

Choose a reason for hiding this comment

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

This reads great, thank you for putting this work together so quickly!

}
}

func TestRemoveFlinkJobExecution(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice test coverage! ✨

return GenerateResourceWithNameAndStatus("", "")
}

func GenerateFlinkJob() meroxa.FlinkJob {
Copy link
Contributor

Choose a reason for hiding this comment

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

I see, I was already wondering how you can mock out a flink job in those unit tests 👀

@janelletavares janelletavares merged commit a4b124e into master May 30, 2023
@janelletavares janelletavares deleted the delete-flink-job branch May 30, 2023 19:32
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.

5 participants