Split integration tests by cloud and allow users to specify credentials #2400

Activity

1. felixwang9817

   added

   kind/featureNew feature or requestNew feature or request

   on Mar 11, 2022
2. github-actions

   assigned

   woop

   on Mar 11, 2022
3. woop commented on Mar 11, 2022

   

   woop

   on Mar 11, 2022

   Last edited by woop

   Member

   More actions

   So @felixwang9817 what I had in mind was something like

   Offline Stores

   offline_store_matrix:
    - bigquery
    - redshift
    - snowflake
    - trino
    - etc
   

   then we would have something like

   (pseudo command)

   pytest test_offline_store_integration.py --offline_store_{{offline_store_matrix}}
   

   Which would test all the offline stores with only the sqlite (or Redis?) online store

   Online Stores

   and then we would have the same thing for all online stores

   pytest test_online_store_integration.py --online_store_{{online_store_matrix}}
   

   which would then test parquet/Dask with all the online stores.

   End to end

   And then we would have a couple of fast running e2e tests. Maybe we only test ones that dont require cloud providers. Something like

   offline_store_matrix:
    - file
    - postgres
   

   online_store_matrix:
   - redis
   - sqlite
   - postgres
   

   pytest test_e2e.py --online_store_{{online_store_matrix}}  --offline_store_{{offline_store_matrix}}
   
4. felixwang9817 commented on Mar 11, 2022

   

   felixwang9817

   on Mar 11, 2022

   CollaboratorAuthor

   More actions

   I agree with the direction of your proposal. My main question: is it safe to eliminate our E2E tests for cloud providers? In your proposal we only run E2E tests for non-cloud providers, and we independently test cloud-based offline and online stores by matching them with a default non-cloud online or offline store. This strikes me as probably more efficient, but would we be comfortable with this?

   I'd also note that I think we should have different tests for local development and PR integration tests. I think your idea of testing all cloud-based offline stores with a sqlite online store is good for local development, but maybe not safe for merging?
5. woop commented on Mar 11, 2022

   

   woop

   on Mar 11, 2022

   Last edited by woop

   Member

   More actions

   My main question: is it safe to eliminate our E2E tests for cloud providers?

   I believe it can be made safe, but I am not sure what guarantees our tests make today. I have specifically added 2 different sets of offline and online stores in the e2e tests, and they should also be in the offline only and online only tests. Meaning it would be very unlikely that a complete e2e test suite (all offline and online stores) would uncover something new. Unless its maybe dependency related or something. As long as we have strict guarantees on the boundaries between offline<>online (the handover), then running all the tests seems wasteful to me.

   What if we followed my proposed direction but moved the rest of the e2e tests to nightly. We can do a full grid at 4am or something?

   I'd also note that I think we should have different tests for local development and PR integration tests.

   Why? This strikes me as a source of pain due to inconsistency
6. achals commented on Mar 11, 2022

   

   achals

   on Mar 11, 2022

   Last edited by achals

   Member

   More actions

   @woop are you proposing that we have this matrix of test to supersede any existing tests, or as a whole new family of tests?

   What about tests for things like AWS Feature Server Lambda?

   Right now, all the universal tests have a provider associated with the test config.

   My proposal is to have a matrix field like:

   matrix:
     provider: [local, gcp, aws, others]
   

   and then have the workflow spawn multiple steps, each step only testing the provider that was assigned to it. This could be done by selecting only the tests corresponding to the provider for that test. By default, we'd run tests for all providers (which is the behavior today).

   I think having a matrix for online and offline stores makes sense, but I'm not sure what happens to other features that are provider-dependent, like stream ingestion in the future. The AWS Feature Server is online store dependent, but not really since it should be able to read from any online store technically speaking.
7. woop commented on Mar 12, 2022

   

   woop

   on Mar 12, 2022

   Last edited by woop

   Member

   More actions

   @woop are you proposing that we have this matrix of test to supersede any existing tests, or as a whole new family of tests?

   I mean as a general replacement for tests that require a combination of an offline and online store where we are having this combinatorial explosion.

   What about tests for things like AWS Feature Server Lambda?

   That is separate.

   provider: [local, gcp, aws, others]

   What I dont understand about this approach is which offline and online stores you are testing together. Are you only testing the defaults with each provider? Would we test e.g Redshift with every kind of online store? What happens if we support Athena as well, where will that be tested? What if we have both Datastore and Bigtable support?

   I'm not sure what happens to other features that are provider-dependent, like stream ingestion in the future

   Why would that be different? We could still have a provider_matrix that tests streaming ingestion if its centered on the provider. What I just want to avoid is testing all online stores, offline stores, and streaming ingestion sources for all PRs.

   PS. It's almost a certainty that we will need to have a compute concept in the future.
8. adchia

   added this to Feast Roadmapon Apr 21, 2022
9. adchia

   added

   priority/p1

   on Apr 21, 2022
10. kevjumba commented on Aug 3, 2022

    

    kevjumba

    on Aug 3, 2022

    Collaborator

    More actions

    This is fixed by a combination of things

    1. @pyalex added markers so that we can test cloud providers individually
    2. I added support for devs to supply their own cloud credentials.
11. kevjumba

    closed this as completedon Aug 3, 2022
12. kevjumba

    moved this to Done in Feast Roadmapon Aug 3, 2022