-
Notifications
You must be signed in to change notification settings - Fork 2.8k
handle table/schema names in relationship suggestions properly (close #876) #879
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Review app available at: https://hge-ci-pull-879.herokuapp.com |
Contributor
|
Review app available at: https://hge-ci-pull-879.herokuapp.com |
Contributor
|
Review app available at: https://hge-ci-pull-879.herokuapp.com |
Contributor
|
Review app available at: https://hge-ci-pull-879.herokuapp.com |
praveenweb
approved these changes
Oct 27, 2018
Contributor
|
Review app https://hge-ci-pull-879.herokuapp.com is deleted |
hasura-bot
pushed a commit
that referenced
this pull request
Jul 25, 2024
…879) This PR updates as many tests as possible that use the custom connector so that the tests run over two versions of the custom connector: 1. The custom connector in the repo, which currently speaks `ndc_models` v0.2.x 2. The custom connector from the past (commit ), which is the last version to speak `ndc_models` v0.1.x This helps us test both the NDC v0.1.x and v0.2.x code paths. When the postgres connector upgrades to v0.2.x, we can use the same approach as in this PR to get the tests to run over multiple versions of the postgres connector too, for much better coverage. This approach with the custom connector will become less useful over time as the v0.1.x connector is not updated and will diverge in data from the v0.2.x connector. The postgres connector is likely to be longer-lasting, as it is more stable. The basic test used for `execute` integration tests is `test_execution_expectation` (in `crates/engine/tests/common.rs`) and it has been extended into a version called `test_execution_expectation_for_multiple_ndc_versions` that takes metadata on a per NDC version basis and then runs the test multiple times, once for each NDC version. This allows one to swap out the DataConnectorLink involved in the test to a different one that points at either the v0.1.x or v0.2.x versions of the connector. The assertion is that both connectors should produce the same results, even if they talk a different version of the NDC protocol. As each version runs, we `println!` the version so that if the test fails you can look in stdout for the test and see which one was executing when it failed. Tests that use the custom connector now use `test_execution_expectation_for_multiple_ndc_versions` and run across both connector versions. Some tests were unable to be used across both version as the data between the two versions has changed. Some tests were modified to avoid the changed data so as to support running across both versions. Any tests that use `test_execution_expectation_legacy` don't run across both versions because those tests aren't backed by the same test implementation as `test_execution_expectation_for_multiple_ndc_versions`. Unfortunately the custom connector doesn't use the standard connector SDK, so it doesn't support `HASURA_CONNECTOR_PORT`. This means that the old connector is stuck on 8101. To work around this, I've moved the current connector port to 8102 instead. Technically we might be able to use docker to remap the ports, but then this binds us into always running the connectors in docker in order to move their ports around, so I avoided that approach. Completes APIPG-703 V3_GIT_ORIGIN_REV_ID: fb0e410ddbee0ea699815388bc63584d6ff5dd70
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #876
Description
Table key in schema is
objectfor tables in non public schema and string for tables public schema. Added a function which will check the type and return the table nameWhat component does this PR affect?
Requires changes from other components? If yes, please mark the components:
Related Issue
Solution and Design
Type
Checklist: