Pass all specs to ingestion by file #154

tims · 2019-03-06T16:59:41Z

Ingestion now takes a --workspace option which should be a path to a directory containing importJobSpecs.yaml file.
This yaml file should align with a new feast.types.ImportJobSpecs protobuf which bundles all the used specs by an import job into a single proto.
The workspace directory is also used to write out the errors if no errors storageSpec is provided.

This importJobSpecs.yaml is be generated by core before starting the job. It is not intended to be written by humans (thought it's useful for testing). It is intended to make it easier for the core api to make all the necessary (and only the necessary) specs available to ingestion. Without Ingestion needing to call back to core. This make deploying feast simpler. It also could make debugging what happened with a job a lot easier.

An alternative approach was to have spec in a separate file, but I think a single file this is easier and neater.

The proto looks like:

message ImportJobSpecs {
  string jobId = 1;
  feast.specs.ImportSpec importSpec = 2;
  repeated feast.specs.EntitySpec entitySpecs = 3;
  repeated feast.specs.FeatureSpec featureSpecs = 4;
  repeated StorageSpec servingStorageSpecs = 5;
  repeated StorageSpec warehouseStorageSpecs = 6;
  StorageSpec errorsStorageSpec = 7;
}

Here is an example of the corresponding yaml:

importSpec: 
  type: file.csv
  options:
    path: /tmp/file.csv
  entities:
    - testEntity
  schema:
    entityIdColumn: id
    timestampValue: 2018-09-25T00:00:00.000Z
    fields:
      - name: timestamp
      - name: id
      - featureId: testEntity.none.testInt64
servingStorageSpecs:
  - id: TEST_SERVING
    type: serving.mock
    options: {}
warehouseStorageSpecs:
  - id: TEST_WAREHOUSE
    type: warehouse.mock
    options: {}
entitySpecs:
  - name: testEntity
    description: This is a test entity
    tags: []
featureSpecs:
  - id: testEntity.day.testInt64
    entity: testEntity
    granularity: DAY
    name: testInt64
    owner: [email protected]
    description: This is test feature of type integer
    uri: https://example.com/
    valueType: INT64
    tags: []
    options: {}

…from a yaml file

… whenever they are set, ignore what the feature specs say

tims · 2019-03-06T17:08:20Z

Also worth noting, ImportJobSpec is not a proto for public consumption, just for communicating between core and ingestion.

It is a great idea for us to find a way for users to define multiple feature specs and an import spec all in one yaml file, but that is a completely independent problem (and would suit a much more concise format than this one is). So let's stay on topic and not get caught on that.

…uch change for one PR

…the importJobSpecs instead of options

…that to ingestion

…use stores with a test

tims · 2019-03-08T15:25:18Z

/hold cancel

tims · 2019-03-08T15:25:52Z

@pradithya ready for review

tims · 2019-03-10T04:29:59Z

/retest

tims · 2019-03-10T04:47:05Z

I original include additional changes that limited to single data store for serving and warehouse, but I removed that to make this easier to review.

Instead: the additional core changes include:

Core can now also be configured with the following additional properties:

feast.store.serving.type = ${STORE_SERVING_TYPE:}
feast.store.serving.options = ${STORE_SERVING_OPTIONS:{}}
feast.store.warehouse.type = ${STORE_WAREHOUSE_TYPE:}
feast.store.warehouse.options = ${STORE_WAREHOUSE_OPTIONS:{}}
feast.store.errors.type = ${STORE_ERRORS_TYPE:}
feast.store.errors.options = ${STORE_ERRORS_OPTIONS:{}}

If present, these will be substituted when a feature spec is applied that does not provide any store id.
The serving and warehouse stores are registered on startup, the same way a use might register a store. So if you only want these stores, you never need to provide any store references in the feature specs.

The following core properties are now mandatory:

feast.jobs.workspace=${JOB_WORKSPACE}

And feast.jobs.coreUri has been removed as it is no longer necessary for ingestion to know the uri of core.

This workspace must point to a directory in core and ingestion have read and write access. Core will create a subdirectory per job and provide that as the ingestion job's workspace directory, then write a importJobSpecs.yaml file to it. Ingestion will read that file to configure start the beam pipeline.

Additionally, if the importErrorsSpecs proto does not include a errorsStorageSpec in it, ingestion will default to writing errors with type "file.json" to the it's workspace/errors directory.

Question for reviewers. Core and ingestion overload the name workspace, for core it means the base path, for ingestion it means the {base path}/{jobId}. Is this acceptable or should we change configuration of core to be baseWorkspace?

tims · 2019-03-10T05:03:10Z

/hold

updating the helm chart

…ts + tests

tims · 2019-03-16T02:45:56Z

/hold

there's an issue where core is not using google-cloud-nio to access the gcs filesystem.

tims · 2019-03-16T03:31:29Z

I'm getting these errors:

   extendedStackTrace: [17]    
   localizedMessage:  "Provider "gs" not installed"    
   message:  "Provider "gs" not installed"    
   name:  "java.nio.file.FileSystemNotFoundException"

   extendedStackTrace: [
    0: {
     class:  "java.nio.file.Paths"      
     exact:  false      
     file:  "Paths.java"      
     line:  147      
     location:  "?"      
     method:  "get"      
     version:  "1.8.0_181"      
    }
    1: {
     class:  "feast.core.util.PathUtil"      
     exact:  false      
     file:  "PathUtil.java"      
     line:  33      
     location:  "classes!/"      
     method:  "getPath"      
     version:  "?"      
    }

This does not make sense to ,me since I've added the google-cloud-nio library and it passes the tests. I can reproduce the error in the tests if I remove it from the pom.xml. :(

Any suggestions for why this might not be included in build?

tims · 2019-03-16T03:35:18Z

out of paranoia I confirmed that it does exist in BOOT-INF/lib/google-cloud-nio-0.83.0-alpha.jar if I unzip the core jar. hmmmm... 🤔

pradithya · 2019-03-16T15:27:12Z

feast.store.serving.type = ${STORE_SERVING_TYPE:}
feast.store.serving.options = ${STORE_SERVING_OPTIONS:{}}
feast.store.warehouse.type = ${STORE_WAREHOUSE_TYPE:}
feast.store.warehouse.options = ${STORE_WAREHOUSE_OPTIONS:{}}
feast.store.errors.type = ${STORE_ERRORS_TYPE:}
feast.store.errors.options = ${STORE_ERRORS_OPTIONS:{}}

Are these mandatory? I am not able to start core without setting them

pradithya · 2019-03-16T16:27:33Z

I got following error when trying to submit job and the workspace is in GCS

Optional[Exception in thread "main" java.lang.UnsupportedOperationException: GCS objects aren't available locally
	at com.google.cloud.storage.contrib.nio.CloudStoragePath.toFile(CloudStoragePath.java:303)
	at feast.ingestion.util.ProtoUtil.decodeProtoYamlFile(ProtoUtil.java:37)
	at feast.ingestion.config.ImportJobSpecsSupplier.create(ImportJobSpecsSupplier.java:45)
	at feast.ingestion.config.ImportJobSpecsSupplier.get(ImportJobSpecsSupplier.java:57)
	at feast.ingestion.ImportJob.mainWithResult(ImportJob.java:121)
	at feast.ingestion.ImportJob.main(ImportJob.java:109)]
	at feast.core.job.direct.DirectRunnerJobManager.runProcess(DirectRunnerJobManager.java:113)

tims · 2019-03-17T00:12:58Z

I got following error when trying to submit job and the workspace is in GCS

Optional[Exception in thread "main" java.lang.UnsupportedOperationException: GCS objects aren't available locally
	at com.google.cloud.storage.contrib.nio.CloudStoragePath.toFile(CloudStoragePath.java:303)
	at feast.ingestion.util.ProtoUtil.decodeProtoYamlFile(ProtoUtil.java:37)
	at feast.ingestion.config.ImportJobSpecsSupplier.create(ImportJobSpecsSupplier.java:45)
	at feast.ingestion.config.ImportJobSpecsSupplier.get(ImportJobSpecsSupplier.java:57)
	at feast.ingestion.ImportJob.mainWithResult(ImportJob.java:121)
	at feast.ingestion.ImportJob.main(ImportJob.java:109)]
	at feast.core.job.direct.DirectRunnerJobManager.runProcess(DirectRunnerJobManager.java:113)

This is strange, because I got a similar error from core, when it was trying to write the specs, you got it ingestion when it was trying to read them.

…res will fail validation if none is registered

tims · 2019-03-17T00:28:06Z

feast.store.serving.type = ${STORE_SERVING_TYPE:}
feast.store.serving.options = ${STORE_SERVING_OPTIONS:{}}
feast.store.warehouse.type = ${STORE_WAREHOUSE_TYPE:}
feast.store.warehouse.options = ${STORE_WAREHOUSE_OPTIONS:{}}
feast.store.errors.type = ${STORE_ERRORS_TYPE:}
feast.store.errors.options = ${STORE_ERRORS_OPTIONS:{}}

Are these mandatory? I am not able to start core without setting them

They are both optional. Just fixed.
A serving store still needs to be registered before feature specs can pass validation

pradithya · 2019-03-18T02:16:19Z

This is strange, because I got a similar error from core, when it was trying to write the specs, you got it ingestion when it was trying to read them.

For me, ImportJobSpec is successfully uploaded to GCS, but ingestion is unable to access it with above error.

Note that, I have to change following line when passing the workspace

    commands.add(option("workspace", workspace.toUri().toString()));

core/src/test/java/feast/core/job/dataflow/DataflowJobManagerTest.java

ingestion/src/main/java/feast/ingestion/config/ImportJobSpecsSupplier.java

core/src/main/java/feast/core/job/direct/DirectRunnerJobManager.java

tims · 2019-03-19T07:47:23Z

/hold cancel

…in gcs

pradithya · 2019-03-21T03:50:08Z

/lgtm

zhilingc · 2019-03-21T03:50:15Z

/approve

feast-ci-bot · 2019-03-21T03:50:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zhilingc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [zhilingc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

tims added 3 commits March 6, 2019 22:13

Ingestion to read all specs into a consolidated ImportJobSpecs proto …

e21690e

…from a yaml file

allow setting only one warehouse and serving store and write to those…

1212947

… whenever they are set, ignore what the feature specs say

all the tests!

e7e0972

feast-ci-bot added the do-not-merge/hold label Mar 6, 2019

tims requested a review from pradithya March 6, 2019 16:59

feast-ci-bot added the size/XXL label Mar 6, 2019

tims requested review from zhilingc and woop March 6, 2019 17:00

clean up redundant test file

6644d43

tims added 6 commits March 7, 2019 16:34

Tweak ImportJobSpecs proto

18afc24

revert to continue support for multiple stores for now as it is too m…

c544ddf

…uch change for one PR

update ingestion to take a workspace option and get error store from …

829084a

…the importJobSpecs instead of options

update core to write importJobSpecs.yaml to a workspace dir and pass …

add0782

…that to ingestion

add on context refresh to initialize default serving store and wareho…

652bf0a

…use stores with a test

write a test to register features and run a mock job

c70a910

feast-ci-bot removed the do-not-merge/hold label Mar 8, 2019

update application.properties

7c601c3

feast-ci-bot added the do-not-merge/hold label Mar 10, 2019

tims added 6 commits March 10, 2019 13:05

update core helm chart

edc2569

validate errors,serving,warehouse storage specs separately for defaul…

e8f29a1

…ts + tests

more tests for validation

bdb01bd

add gcs File support to core

8c2d501

update readme with new chart options

48ef3a2

fix readme: bigtable, not postgres

25fef01

tims added 2 commits March 15, 2019 19:24

fix type in helm charts

939d8dd

fix helm chart once more

6cef154

feast-ci-bot added the do-not-merge/hold label Mar 16, 2019

add test to ensure gcs filesystem is supported

6cf4c35

tims added 2 commits March 16, 2019 17:22

add test and ensure support for no warehouse

0c894f4

test to ensure no default serving store is required, but adding featu…

bc638ee

…res will fail validation if none is registered

add dirtiesContext to spring boot application tests

2a47a34

pradithya reviewed Mar 18, 2019

View reviewed changes

core/src/test/java/feast/core/job/dataflow/DataflowJobManagerTest.java Show resolved Hide resolved

pradithya reviewed Mar 18, 2019

View reviewed changes

ingestion/src/main/java/feast/ingestion/config/ImportJobSpecsSupplier.java Show resolved Hide resolved

pradithya reviewed Mar 18, 2019

View reviewed changes

core/src/main/java/feast/core/job/direct/DirectRunnerJobManager.java Outdated Show resolved Hide resolved

pass workspace as uri

1d2dd92

feast-ci-bot removed the do-not-merge/hold label Mar 19, 2019

tims added 2 commits March 20, 2019 17:57

proto util tests and fix bug where it couldn't load protos from yaml …

1d9199a

…in gcs

compile test protos as needed

d8e8736

feast-ci-bot added the lgtm label Mar 21, 2019

feast-ci-bot added the approved label Mar 21, 2019

feast-ci-bot merged commit 6646402 into feast-dev:master Mar 21, 2019

zhilingc mentioned this pull request Mar 22, 2019

Remove granularity #162

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pass all specs to ingestion by file #154

Pass all specs to ingestion by file #154

tims commented Mar 6, 2019 •

edited

Loading

tims commented Mar 6, 2019

tims commented Mar 8, 2019

tims commented Mar 8, 2019

tims commented Mar 10, 2019

tims commented Mar 10, 2019

tims commented Mar 10, 2019

tims commented Mar 16, 2019

tims commented Mar 16, 2019

tims commented Mar 16, 2019

pradithya commented Mar 16, 2019

pradithya commented Mar 16, 2019

tims commented Mar 17, 2019

tims commented Mar 17, 2019

pradithya commented Mar 18, 2019

tims commented Mar 19, 2019

pradithya commented Mar 21, 2019

zhilingc commented Mar 21, 2019

feast-ci-bot commented Mar 21, 2019

Pass all specs to ingestion by file #154

Pass all specs to ingestion by file #154

Conversation

tims commented Mar 6, 2019 • edited Loading

tims commented Mar 6, 2019

tims commented Mar 8, 2019

tims commented Mar 8, 2019

tims commented Mar 10, 2019

tims commented Mar 10, 2019

tims commented Mar 10, 2019

tims commented Mar 16, 2019

tims commented Mar 16, 2019

tims commented Mar 16, 2019

pradithya commented Mar 16, 2019

pradithya commented Mar 16, 2019

tims commented Mar 17, 2019

tims commented Mar 17, 2019

pradithya commented Mar 18, 2019

tims commented Mar 19, 2019

pradithya commented Mar 21, 2019

zhilingc commented Mar 21, 2019

feast-ci-bot commented Mar 21, 2019

tims commented Mar 6, 2019 •

edited

Loading