vmware / versatile-data-kit Goto Github PK

Add Licenses whitelist & validation on PR Gitlab check.

• SDK uses https://github.com/raimon49/pip-licenses, that supports allow-only verifying licenses match a list of permitted ones.
• Control Service uses https://github.com/jk1/Gradle-License-Report plugin, that supports licenses file (whitelisting) to be verified by task checkLicense.

Find Dependencies licensing page on generating dependencies license report (, then cleanup sections after enabling license verification?).
Add a sentence about license check in CONTRIBUTING.md.

What is the feature request? What problem does it solve?
See DeploymentProgress measurables:

Issue is, JobDeployment DTO contains data about data job name only:

API clearly defines that a unique data job asset ID is composite of team and job name.

This causes any reports relying on correct owner team not to work properly.

Suggested solution
Add team name to JobDeployment DTO

Acceptance Criteria

• When I run Control service with telemetry hook enabled (use online webhook test like http://ptsv2.com) and deploy a job I can see a json payload which shows deployment progress and it contains team name as well now.

What is the feature request? What problem does it solve?
If a plugin fails to load currently we'd get a stack trace and the only way to find out which plugin has failed to load is to check out the stacktrace and hope to know the namespace used by the plugin. This is not very user-friendly.
We need a better way to signal to users that the plugin has not loaded.

The fail to load plugin also may cause vdk to stop working. So we need to consider to quarantine such plugins

Suggested solution
Have clear error message in the VDK error format (what,why,consequences, countermeasures) which also say which plugin fail to load , what are the consequences (VDK Is broken) and what user can do (uninstall the plugin, reinstall it, upgrade it)

What is the feature request? What problem does it solve?
A clear and concise description of what problem needs to be solved.

Suggested solution
A clear and concise description of what you want to happen.

Additional context
Add any other context or screenshots about the feature request here.

What is the feature request? What problem does it solve?
A clear and concise description of what problem needs to be solved.

Suggested solution
A clear and concise description of what you want to happen.

Additional context
Add any other context or screenshots about the feature request here.

What is the feature request? What problem does it solve?
pip install quickstart-vdk fails on Python 3.10 with a very uninformative message. Also README.md says that vdk requires "Python 3.7+" which implies that Python 3.10 is supported.

Suggested solution
Test vdk on Python 3.10 and, then, update the setup.cfg metadata to include 3.10.

Additional context

What is the feature request? What problem does it solve?

Contributors to this project would find it useful to have a local testing environment that can be started and configured without too much hassle. E.g if a developer has knowledge only on control service they needn't learn how the helm charts and/or vdk-core work to be able to deploy, use (as if installed in cloud) and remove the tool in a local environment.

Suggested solution

The current installation of vdk-server comes with a predefined version of the control-service. Ideally, we want to be able to deploy, schedule and run data jobs on a locally running cluster such as vdk server and get the same behavior as if the data job was running in a cloud instance.

Add a feature that enables us to hook a locally running version of the control service (started by a script, or an IDE such as Eclipse or IntelliJ etc..) to a running vdk-server installation. Change the default settings in application.properties to make this connection hassle free and expect the default settings of the vdk-server installed cluster.

Additional context

This request was discussed during a team technical alignment meeting and the overall consensus was that this is a worthwhile feature.

What is the feature request? What problem does it solve?

Prior to 7.0 click would convert flag value to enum type. But it was changed (to be consistent) and returns string now. 

vdk-control cli (login, deploy commands) expect click to convert flag values to enum but it does to string, so we need to fix it 

See also pallets/click#1960

Suggested solution

Use only strings and EnumName.value which returns string representation of the enum value where comparison is necessary 
In the end, it is expected that we can support click 8.0

What is the feature request? What problem does it solve?
Often users would need to export result of some query to CSV file so they can send it to a colleague or to analyze it with other excel files they already have or to import it somewhere else. This is so frequent that a lot of tools have an option to export to csv (it's standard feature of any SQL client for example).

Suggested solution
We'd like to have generic command in vdk called vdk export-csv which can export from the default database to a local file.

We can extend vdk-csv by adding that command alongside existing ingest-csv.

What is the feature request? What problem does it solve?
We have a diagram that describes the relationships between different API components - https://raw.githubusercontent.com/wiki/vmware/versatile-data-kit/vdk-data-job-lifecycle-state-diagram.png

The purpose of the diagram is to help users understand better how the API works and how it is expected to be used (describing the usual data engineering work flow)

It would be good to describe how Execution API fits. If it is too much one diagram we can split it in two.

What is the feature request? What problem does it solve?
A clear and concise description of what problem needs to be solved.

Suggested solution
A clear and concise description of what you want to happen.

Additional context
Add any other context or screenshots about the feature request here.

What is the feature request? What problem does it solve?
singer.io (https://github.com/singer-io/getting-started) is a great open-source project that provides connectors from almost any place:
"Singer is an open source standard for moving data between databases, web APIs, files, queues, and just about anything else you can think of. The Singer spec describes how data extraction scripts — called “Taps” — and data loading scripts — called “Targets” — should communicate using a standard JSON-based data format over stdout. By conforming to this spec, Taps and Targets can be used in any combination to move data from any source to any destination."

VDK provides ingestion API which gives features lik3 parallelism, buffering, asynchronous,etc. would work very well together and would allow users in a practically codeless way to ingest data from any place (using single io taps) to a lot of destinations (using possibly singer.io targets) at very decent scale.

Suggested solution
Create plugin vdk-ingest-singer which would provide that integration
Define data source interface so user can easy configure which is the source (aka singer tap)
maybe something like this

job_input.send_object_for_ingestion(singer.tap.oracle(host=xxx,post=yyy,user=...), method=kafka) 

Or entirely congiuration driven (yaml)

source:  
  type: orclae
  host: ...
  ....
  method: kafka 
  target: kafka_uri

We need POC/spike first.
TODO: link to feature spec for details

What is the feature request? What problem does it solve?
As a user I'd like to find out what plugins can I install? What do they do ?
Currently those are listed in https://github.com/vmware/versatile-data-kit/tree/main/projects/vdk-core/plugins

This is not a terrible solution. But we can be more user-friendly.

Suggested solution
Something like https://docs.pytest.org/en/latest/reference/plugin_list.html might not be bad idea.

But also we may create new plugin vdk-plugin-manager
And it would introduce vdk plugin --list command and/or vdk plugin --search to search and list plugins
and vdk plugin --name name-of-plugin --info (which will print basically the readme of the plugin). That might be easier for users relying on CLI (where plugins usually are installed).

Having UI with that info would be useful as well though.

What is the feature request? What problem does it solve?

Currently, in VDK Control Service helm charts the following params are set:
[https://github.com/vmware/versatile-data-kit/blob/main/projects/control-service/projects/helm_charts/pipelines-control-service/values.yaml]

deploymentDockerRegistryUsername
deploymentDockerRegistryPassword

They are used for authentication when the job image is pushed to the registry on deploy, but they are not applied when the image is pulled from the registry on job execute. This is why currently deployments configured against external container registries which require authentication, cannot execute jobs (they fail to pull the job image from the registry).

While fixing this, please store these params as a secret, because now they are visible in the data job pod yaml.

Suggested solution

Update [https://github.com/vmware/versatile-data-kit/blob/main/projects/control-service/projects/helm_charts/pipelines-control-service/templates/secrets.yaml] to keep docker registry credentials

add secret name - https://github.com/vmware/versatile-data-kit/blob/main/projects/control-service/projects/helm_charts/pipelines-control-service/templates/deployment.yaml as env. variable

JobImageDeployer class in control serivce (using @value spring autowiring) read the variable name
updateCronJob - set imagePullSecrets

Acceptance criteria

Data Job can be executed when Control Service is configured against a private container registry (authentication when pulling the job image passes successfully).

Docker registry username and password are not visible in the data job pod yaml.

What is the feature request? What problem does it solve?
A clear and concise description of what problem needs to be solved.

Suggested solution
A clear and concise description of what you want to happen.

Additional context
Add any other context or screenshots about the feature request here.

What is the feature request? What problem does it solve?
Tagging releases is very standard practice - https://docs.github.com/en/repositories/releasing-projects-on-github/managing-releases-in-a-repository and
They can be easily seen in https://github.com/vmware/versatile-data-kit/releases/new
It would make it easier for users to navigate what artifacts are available. And also easier to track change log/ release notes on what has changed so user can decide to upgrade or not.

Suggested solution
Not sure, We need some kind of process or automation:
E.g user creat tag 2.1 (from 2.0) -> this automatically updates (automated commit) the version to 2.1 in the version.txt file (or where the version is) and create new entry in CHANGELOG updating the data

Patch release are not tagged and are automated on each commit with successful CI pipeline

Additional context
Add any other context or screenshots about the feature request here.

What is the feature request? What problem does it solve?
KubernetesService has gotten too heavy, complex and unusable. It needs a redesign to simplify, externalise some code, optimise and refine readability. Define clear interface surface.
 
Acceptance criteria
Proposal, stories & estimate;
Move service and (externalised) inner/nested classes to dedicated com.vmware.taurus.kubernetes package;
Review methods for duplications, their interface surface (parameters, orders, static/non-static). Make them readable and reusable

What is the feature request? What problem does it solve?

VDK users Data Job deployments might fail if Git has performance problems and takes too much time to upload the data job code. This can happen with varying frequency - not too often since Git server would usually be stable. Still it is an external component which we connect through a network. The network can be slow, the server can be slow , we should account for those.

Suggested solution

Check if we have retries for this problem and consider adding them or modifying their number and frequency.

The change will happen in the control service project

• Link to the project directory.

• Link to development setup of the project .

• The change would be in the Control Service Java application here

• The class name that most relevant is JobUpload.java

• Here is an sample diagram of how upload functionality works

test issue

What is the feature request? What problem does it solve?
Currently the Termination Message Writer plugin writes the vdk-core version to the termination message. We need the version of the main distribution of vdk that aggregates different plugins and vdk-core (e.g quickstart-vdk, company-x-vdk).
From user point of view, this is the main version they'd be using and they know. Also from the vdk distribution version we can see the version of the plugins potentially.

Suggested solution
We can re-use PACKAGE_NAME -

What is the feature request? What problem does it solve?
Currently the vdk-core CI/CD does not have a pre-release test. Several times this led to regressions. We need to introduce pre-release test step that executes vdk-heartbeat. (We can consider a generic step to be executed on change in all projects (vdk-core, control-service, etc.))

Acceptance criteria*
We should have working pre-release test step in the pipeline.

What is the feature request? What problem does it solve?
A clear and concise description of what problem needs to be solved.

Suggested solution
A clear and concise description of what you want to happen.

Additional context
Add any other context or screenshots about the feature request here.

What is the feature request? What problem does it solve?
Big Query is a pretty popular database by Google and it would be very useful for vdk jobs to be able to natively executed queries against it in a managed way.

Suggested solution
We need vdk-bigquery plugin similar to the ones we really have . The integration is pretty straightforward. Using python big query client (https://googleapis.dev/python/bigquery/latest/index.html):

See https://github.com/vmware/versatile-data-kit/pull/197/files for vdk-snowflake
or https://github.com/vmware/versatile-data-kit/pull/361/files for vdk-greenplum

as examples. The code would be pretty much the same

Marks the method as private.

Originally posted by @gageorgiev in #168 (comment)

Isn't single underscore used for private functions? and double underscore used for name mangling?
https://docs.python.org/3/tutorial/classes.html#private-variables

What is the feature request? What problem does it solve?

If the user has set some params from the datajobTemplate section of the VDK deployment helm chart and forgotten to enable it. It will simply not work silently.

Suggested solution
If the user has set some params from the datajobTemplate section of the VDK deployment helm chart, add a warning in the log if datajobTemplate.enabled is set to false. This will help users who want to use some template params in their deployment, understand why their changes are not applied (if they forgot to enable data job template params).

What is the feature request? What problem does it solve?
We need clearly defined coding standard for the project. There are some recommendation that are passed using worth of mouth but that is not very clear even to us existing contributors let alone new ones

Suggested solution
Create wiki with clear coding standards for java,python,helm charts

helm charts use same standard as bitnami https://github.com/bitnami/charts/tree/master/bitnami
Python is based on PEP 8
Java - ? Google style guide, We can reuse https://github.com/maltzj/google-style-precommit-hook
Error handling https://github.com/vmware/versatile-data-kit/blob/main/projects/control-service/CONTRIBUTING.md#error-handling
Logging

What is the feature request? What problem does it solve?

Currently we only have a single state diagram and API descriptions https://github.com/vmware/versatile-data-kit/blob/main/projects/control-service/projects/model/apidefs/datajob-api/api.yaml which is not that user friendly.
it is not always clear how it can be used . There are some examples but it would be better if we have use-case specific examples and documentation .

Suggested solution
We need to provide more detailed REST API documentation with examples of what can be achieved, it can be in the form of very basic user scenarios or basic wireframes what can be done in UI using the REST API.

Describe the bug
A clear and concise description of what the bug is.

Steps To Reproduce
Steps to reproduce the behavior:

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
If applicable, add screenshots to help explain your problem.

Version (please complete the following information):

• OS: [e.g. Windows]
• Version (e.g print output of vdk version)

Additional context
Add any other context about the problem here. Consider attaching logs if there are any.

What is the feature request? What problem does it solve?

From a design point, each plugin is independent of one another and of vdk-core.
Each plugin depends on vdk-core so they know aboutn vdk-core.

But vdk-core does not depend on any plugin and it should not even know about plugin existence.

Currently, plugins are put in vdk-core project folder. It was done mostly when vdk-core was moved from its previous repository. But it would be better if there are further decoupled as this may cause confusion to users and developers.

Suggested solution
Move plugins directory to top-level projects directory.
From projects/vdk-core/plugins (now) to projects/plugins

Add the include in vdk-core https://github.com/vmware/versatile-data-kit/blob/main/projects/vdk-core/.gitlab-ci.yml#L4

should be moved to the top level .gitlab-ci.yaml file -> https://github.com/vmware/versatile-data-kit/blob/main/.gitlab-ci.yml

What is the feature request? What problem does it solve?
Currently, if a data job execution exceeds the allowed memory quota, the pod of the execution is immediately killed by Kubernetes and restarted. Which in turn usually results in a similar failure. Ultimately (after 3 re-attempts), the Kubernetes job fails, which is currently interpreted as Platform Error.

Suggested solution
Change the logic that determines the error type to report these failures as User Error. One way to determine that a pod has been terminated due to OOM, is to describe it and check the LastState of the container:

Containers:
    <name>:
    ...
    Last State:     Terminated
        Reason:       OOMKilled
        Exit Code:    137
        Started:      Mon, 18 Oct 2021 16:31:40 +0300
        Finished:     Mon, 18 Oct 2021 16:39:05 +0300

Acceptance criteria
Jobs executions that are terminated due to OOM are classified as User Error.

What is the feature request? What problem does it solve?

We should be cleaning up everything after data job is deleted. 

Currently we are leaving the docker images back. This will create a problem down the line as the number of jobs increase or we have often new and deleted job (e.g in tests).
Still this should be configurable as image may be used for build reproducibility.

Suggested solution

1. Step 1. Add feature flag "remove_container_image_on_job_deletion" in the helm chart and pass it as env variable to control service deployment .
2. Step 2. If the feature flag is true when job is deleted in on job delete, delete all images ?
   2. 1. We probably should add deleteDeployments to DeploymentService

The deletion of image should rely only on OCI Specifications: https://github.com/opencontainers/distribution-spec/blob/main/spec.md

What is the feature request? What problem does it solve?
If a k8s job is running for too long (longer than what is specified by activeDeadlineSeconds (currently, 36 hours), it will be terminated. This is classified as a Platform Error because the job failed without a termination message. However, this could happen if the job is not optimized to complete in a timely manner. Classify such cases as User Errors and notify the user accordingly.

Acceptance criteria: Job executions that exceed the deadline are classified as User errors.

What is the feature request? What problem does it solve?
All cursor pre-execute handlers, were not always triggered.
Managed connection and cursor redesigned, to enable both ways of executing queries - via job_input.execute_query() and job_input.get_managed_connection().cursor().execute().

Suggested solution
Address comments of PR #388

Acceptance criteria
merge feature

What is the feature request? What problem does it solve?
Currently, all jobs use the same vdk version in which to run. This makes it easier to maintain thousands of jobs as they are uniform.
But if we want to do a canary release of vdk by rolling out the initial version of vdk to a subset of users as an initial test.
Or we want to do A/B testing with different flavors of vdk. We need to be able to override the vdk version for subset of jobs.

Another use-case is when vdk introduces a major release (aka breaking change). We'd need to enable users to use both previous and new major releases concurrently so they can gradually switch to the new release.

Suggested solution
During deploy we can set not only job version (as we do not) but also vdk version (by default it's left empty so it uses latest)
This would all us to override the vdk version per deployment basis

Acceptance Criteria

• I can deploy a single job with a specific (possibly non-officially released) version of vdk in cloud runtime. While all the other jobs use the common version.

What is the feature request? What problem does it solve?
We can accidentally break quickstart-vdk (which is the first distribution user would use) if we make a change in vdk-core or some plugin. It would be good to have some verification before releasing a plugin or vdk-core or quickstart-vdk. vdk-heartbeat is the test we use for such verification so we may use it.

Currently, plugin versions are not fixed/hard-coded in quickstart-vdk and when we upgrade it can break.

Suggested solution
Run vdk-heartbeat on release of quickstart-vdk
On release of vdk-core or plugin we may consider running the same tests.
Alternatively we can pin the versions in quickstart-vdk.

Steps:

• configure vdk-heartbeat with specific vdk version (if vdk version set - vdk --update)
• ci/cd jobs in vdk-core - release candidate image and rc distribution
• run release acceptance with them
• real release

Refer to supercollider's ci/cd - [https://gitlab.eng.vmware.com/product-analytics/data-pipelines/vdk/-/blob/master/.gitlab-ci.yml#L72]

It depends on [https://github.com//issues/377]

Acceptance criteria:

vdk-heartbeat runs successfully before vdk-core and quickstart-vdk release

What is the feature request? What problem does it solve?
We are publishing Control Service integration tests image during the CI/CD release stage without even testing it against the current version of Control Service. This can lead to the publication of integration tests image that is not compatible with the current Control Service version.

Suggested solution
Change the CI/CD as follows:

1. Publish the integration tests image together with the Control Service image during the publish_artifacts stage.
2. In order to verify the integration tests image produced by publish_artifacts stage is working properly we should run the it during the pre_release_test stage. (https://gitlab.com/gitlab-org/gitlab-runner/-/issues/112)
3. Tag integration tests and Control Service images with the appropriate semantic version during the release stage.

Very recently pluggy had release major version : https://pluggy.readthedocs.io/en/stable/changelog.html#pluggy-1-0-0-2021-08-25
 

Pluggy is one of the 2 main libraries we require for vdk-core (other being click) and as it's such a core component it's very important to keep it up to date and support all versions. 
As vdk-core is going to be included in all vdk plugins and all data jobs that means we need to make sure we do not restrict our users in terms. of what they can use. 

That's why it was unpinned initially (the version was not fixed). The major upgrade naturally broke some things and I pinned the version to 0.* . But we need to support 1.0 as well as 0.* ASAP. 

Acceptance Criteria: 

• vdk-core and vdk-control-cli works against all versions of pluggy (0.x or 1.x)

What is the feature request? What problem does it solve?

Contributors to this project would find it useful to have a local testing environment that can be started and configured without too much hassle. E.g if a developer has knowledge only on vdk-core they needn't learn how the helm charts and/or control-service work to be able to deploy, use (as if installed in cloud) and remove the tool in a local environment.

Suggested solution

The current installation of vdk-server comes with a pre-defined version of vdk-quickstart and the control-service. Ideally, we want to be able to deploy, schedule and run data jobs on a locally running cluster such as vdk server and get the same behavior as if the data job was running in a cloud instance.

Add a feature that enables us to use a local version of the vdk package that data jobs will run on during installation of vdk-server.

Additional context

This request was discussed during a team technical alignment meeting and the overall consensus was that this is a worthwhile feature.

What is the feature request? What problem does it solve?
Currently there is no easy way to retrieve job success rate. We want to expose this metric thought the GraphQL API so that users can retrieve this data.

Suggested solution

1. Modify the GraphQL Job Deployment schema: add separate fields (numberOfsuccess/executionsSucceeded, numberOfFailures/executionsFailed) at deployment level.

2. In order to reduce the size of the dateset on which the calculations will be made we should calculate these metrics during the Post Pagination phase (populateDataJobsPostPagination).

   Sample query for success rate calculation (data_job_execution.status 2 is Finished; data_job_execution.status 3 is Failed):

   SELECT COUNT(status), status, job_name FROM data_job_execution
   WHERE status IN (2, 3) AND job_name IN ('job_name1', 'job_name2') 
   GROUP BY status, job_name
   

3. Calculate the executionsSucceeded and executionsFailed only if they are present in the request (GraphQL query).

4. Provide proper unit tests

5. Provide proper Integration tests

Additional context
Sample GraphQL query:

{{ baseUrl  }}/data-jobs/for-team/no-team-specified/jobs?query={
  jobs(pageNumber: 1, pageSize: 100, filter: [{property: "jobName", sort: ASC}]) {
    content {
      jobName
      config {
        team
        description
        schedule {
          scheduleCron
        }
      }
      deployments {
        enabled
        status
        executionsSucceeded,
        executionsFailed,
        executions(pageNumber: 1, pageSize: 100, filter: []) {
          id
          startTime
        }
      }
    }
  }
}

Sample GraphQL response:

{
  "errors": [],
  "data": {
    "content": [
      {
        "jobName": "tms-staging-vdk-1633680894",
        "config": {
          "team": "taurus",
          "description": "",
          "schedule": {
            "scheduleCron": "* * * * *"
          }
        },
        "deployments": [
          {
            "enabled": true,
            "status": null,
            "executionsSucceeded": 100,
            "executionsFailed": 10,
            "executions": [
              {
                "id": "tms-staging-vdk-1633680894-1634638440",
                "startTime": "2021-10-19T10:14:47Z"
              },
              {
                "id": "tms-staging-vdk-1633680894-1634638500",
                "startTime": "2021-10-19T10:15:47Z"
              },
              {
                "id": "tms-staging-vdk-1633680894-1634638560",
                "startTime": "2021-10-19T10:16:47Z"
              }
            ]
          }
        ]
      }
    ],
    "totalItems": 0,
    "totalPages": 0
  }
}

Acceptance criteria
The GraphQL API should provide an ability to select number of succeeded and failed executions.

What is the feature request? What problem does it solve?

Currently, we are using the log configuration to determine if the execution is Cloud or Local. We need to refactor this so that we have a dedicated env variable or method in vdk-core and not rely on the logging config.

We should not be using log configuration to determine unrelated concern like where it is executed we may want to add other log configuration types

Suggested solution*
We can introduce new variable execution environment which can be local or cloud and in control service we would set it to always be cloud.

Alternatively, we can introduce some new method in vdk core and abstract away the logic - in this case it would not be that big of a problem to use log configuration type because it is only one place to change

What is the feature request? What problem does it solve?
Currently, if I run a VDK command which requires me to be authenticated, and I am not, the execution stops with an error message stating that I need to be authenticated. In some cases the error is crpytic and even that is not clear.

Then I need to manually authenticate and re-try the command (I need to figure it out). This is 3 manual steps to get successful execution. If token expires quickly (e.g 30 minutes) this can be pretty frequently

Suggested solution
VDK should attempt to authenticate me automatically instead of printing an exception. Possibly automatically opening my browser so I can login (or if I am already logged in in SSO , then do nothing).

Existing workaround
If a user using API Token authentication, they can login once vdk login -t api-token or set VDK_API_TOKEN and it would no longer require login as long as the API token reminds valid (which can be months).

Presently, Git access is being yaml configurable  using properties:

deploymentGitUsername: ""
deploymentGitPassword: ""
uploadGitReadWriteUsername: ""
uploadGitReadWritePassword: "" 

SSH key support for RSA, ECDSA, or ED25519 key fingerprint needed. Might potentially be stored in k8s secret, for JGit to use.

We need to enable Git access by configuring a Git Deploy Key.

• 1
• 2