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

Allow registration of feature without warehouse store #80

Merged
merged 2 commits into from
Jan 17, 2019

Conversation

pradithya
Copy link
Collaborator

Fix #5

@pradithya
Copy link
Collaborator Author

/assign @zhilingc
/assign @woop

@pradithya
Copy link
Collaborator Author

pradithya commented Jan 17, 2019 via email

@zhilingc
Copy link
Collaborator

That's to avoid NPE

On Thu, 17 Jan 2019, 12:46 Chen Zhiling @.*** wrote: @.**** requested changes on this pull request. ------------------------------ In core/src/main/java/feast/core/validators/SpecValidator.java <#80 (comment)>: > @@ -138,13 +140,17 @@ public void validateFeatureSpec(FeatureSpec spec) throws IllegalArgumentExceptio checkArgument( Arrays.asList(SUPPORTED_SERVING_STORES).contains(servingStore.get().getType()), Strings.lenientFormat("Unsupported serving store type", servingStore.get().getType())); - checkArgument( - warehouseStore.isPresent(), - Strings.lenientFormat("Warehouse store with id %s does not exist", warehouseStoreId)); - checkArgument( - Arrays.asList(SUPPORTED_WAREHOUSE_STORES).contains(warehouseStore.get().getType()), - Strings.lenientFormat( - "Unsupported warehouse store type", warehouseStore.get().getType())); + + if (!NO_STORE.equals(warehouseStoreId)) { should this be the other way around? !warehouseStoreId.equals(NO_STORE) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#80 (review)>, or mute the thread https://github.com/notifications/unsubscribe-auth/AD1i5_KAMI-V3Nko6wOthn09jkXOaQ_Mks5vEAA3gaJpZM4aEa7O .

Don't warehouseStoreId and servingStoreId default to empty strings before that check is done?

@zhilingc
Copy link
Collaborator

/lgtm

@feast-ci-bot feast-ci-bot removed the lgtm label Jan 17, 2019
@zhilingc
Copy link
Collaborator

/lgtm

@woop
Copy link
Member

woop commented Jan 17, 2019

/approve

@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: woop

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:

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

@feast-ci-bot feast-ci-bot merged commit 9215a9e into feast-dev:master Jan 17, 2019
Yanson pushed a commit to Yanson/feast that referenced this pull request Jul 29, 2020
- Adapted custom code for upstream changes in release 0.6.0:
  - FeatureSets are delivered to Ingestion Job through Kafka (feast-dev#792)
     - Closes KE-982 (*Spark job fails when too many featureset are provided in the JSON parameter*)
  - Enable isort for Python SDK 843 
  - Enable linting and formatting for e2e tests 832 

- Cleaned up Python SDK code for downloading files vs. directories
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants