Do not wait the end of a reset to return an update #17591
Merged
Conversation
This file contains 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
github-actions
bot
added
area/platform
benmoriceau
changed the title
benmoriceau
requested review from
alovew,
colesnodgrass,
gosusnp and
jdpgrailsdev
|
Hmm I don't know why the build didn't run. |
|
I tested the Basic and Advance Acceptance Test locally. They passed |
|
I relaunched the test, I think that the error is transient |
gosusnp
approved these changes
Oct 5, 2022
| @@ -642,7 +643,7 @@ void testUpdateConnectionWithUpdatedSchemaLegacy() throws JsonValidationExceptio | |||
| when(configRepository.getAllStreamsForConnection(expected.getConnectionId())).thenReturn(connectionStreams); | |||
|
|
|||
| final ManualOperationResult successfulResult = ManualOperationResult.builder().jobId(Optional.empty()).failingReason(Optional.empty()).build(); | |||
| when(eventRunner.synchronousResetConnection(any(), any())).thenReturn(successfulResult); | |||
| when(eventRunner.resetConnection(any(), any(), anyBoolean())).thenReturn(successfulResult); | |||
| when(eventRunner.startNewManualSync(any())).thenReturn(successfulResult); | |||
There was a problem hiding this comment.
nit: Comment also applies for the other tests: I think we no longer need this mock, can we remove it?
There was a problem hiding this comment.
@gosusnp The build is green, I'll merge/deploy and address your comments in another PR
| @Override | ||
| public void resetConnectionAndSkipNextScheduling() { | ||
| if (workflowState.isDoneWaiting()) { | ||
| workflowState.setCancelledForReset(true); |
There was a problem hiding this comment.
For my understanding, setCancelledForReset means that we cancel the current run and start a reset?
benmoriceau
added a commit
that referenced
this pull request
Oct 5, 2022
This reverts commit b4fd1ea.
benmoriceau
added a commit
that referenced
this pull request
Oct 5, 2022
letiescanciano
added a commit
that referenced
this pull request
Oct 6, 2022
…vation * master: (26 commits) supply a source id for schema discovery in connector integration tests (#17662) Source Iterable: Add permission check for stream (#17602) Moving TrackingClientSingleton.initialize into the bean itself (#17631) Handle null workspace IDs in tracking/reporting methods gracefully (#17641) Bump Airbyte version from 0.40.11 to 0.40.12 (#17653) Revert "Do not wait the end of a reset to return an update (#17591)" (#17640) Standardize HttpRequester's url_base and path format (#17524) Create geography_type enum and add geography column in connection and workspace table (#16818) airbyte-cron: update connector definitions from remote (#16438) Do not wait the end of a reset to return an update (#17591) Remove redundant title labels from connector specs (#17544) Updated GA4 status support large schema discovery (#17394) 🪟 🐛 Fixes connector checks not properly ending their loading state (#17620) 🪟🧪 [Experiment] add hideOnboarding experiment (#17605) Source Recharge: change releaseStage to GA (#17606) Source Recharge: skip stream if 403 received (#17608) remove sonar-scan workflow (#17609) Mark/tables should be full width on all pages (#17401) Auto fail all workfow if there is a Versioning issue (#17562) ...
benmoriceau
added a commit
that referenced
this pull request
Oct 6, 2022
benmoriceau
added a commit
that referenced
this pull request
Oct 7, 2022
letiescanciano
added a commit
that referenced
this pull request
Oct 7, 2022
…vation * master: (32 commits) fixed octavia position and z-index on onboarding page (#17708) Revert "Revert "Do not wait the end of a reset to return an update (#17591)" (#17640)" (#17669) source-google-analytics-v4: use hits metric for check (#17717) Source linkedin-ads: retry 429/5xx when refreshing access token (#17724) 🐛 Source Mixpanel: solve cursor field none expected array (#17699) 🎉 8890 Source MySql: Fix large table issue by fetch size (#17236) Test e2e testing tool commands (#17722) fixed escape character i18n error (#17706) Docs: adds missing " in transformations-with-airbyte.md (#17723) Change Osano token to new project (#17720) Source Github: improve 502 handling for `comments` stream (#17715) #17506 source snapchat marketing: retry failed request for refreshing access token (#17596) MongoDb Source: Increase performance of discover (#17614) Testing tool commands for run scenarios (#17550) Kustomize: Missing NORMALIZATION_JOB_* environment variables in stable-with-resource-limits overlays (#17713) Fix console errors (#17696) Revert: #17047 Airbyte CDK: Improve error for returning non-iterable from connectors parse_response (#17707) #17047 Airbyte CDK: Improve error for returning non-iterable from connectors parse_response (#17626) 📝 Postgres source: document occasional full refresh under cdc mode (#17705) Bump Airbyte version from 0.40.12 to 0.40.13 (#17682) ...
jhammarstedt
pushed a commit
to jhammarstedt/airbyte
that referenced
this pull request
Oct 31, 2022
* Directly save after update * Fix existing tests * Rm unused * Format * Add test t * Format
jhammarstedt
pushed a commit
to jhammarstedt/airbyte
that referenced
this pull request
Oct 31, 2022
…#17591)" (airbytehq#17640) This reverts commit b4fd1ea.
jhammarstedt
pushed a commit
to jhammarstedt/airbyte
that referenced
this pull request
Oct 31, 2022
…irbytehq#17591)" (airbytehq#17640)" (airbytehq#17669) This reverts commit 77084be.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What
return the update endpoint without having to wait for a reset to finish running.
Important files to review
ConnectionManagerWorkflow.java-> add a new signalWorkflowState.java-> Add a new state that allow to continue the new run without waiting for the schedulingConnectionManagerWorkflowImpl.java-> User the new stateTemporalClient.java-> Remove the synchronous reset and call the new signal if needed.Address #16178