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

Replace WorkerPlane with Micronaut environments #17286

Merged
merged 3 commits into from
Sep 28, 2022

Conversation

jdpgrailsdev
Copy link
Contributor

What

  • Replace the custom environment variable used to control the operation mode of the airbyte-workers service with a Micronaut-based environment.

How

  • Removes the WorkerPlane enumeration
  • Updates all conditional code to check the Micronaut environment to see if the required mode is present
  • Defines constants for the environments.

Recommended reading order

  1. WorkerMode.java
  2. ApplicationBeanFactory.java
  3. ApiClientBeanFactory.java
  4. ApplicationInitializer.java
  5. .env

Cloud will need to be updated after merge to ensure that it uses the proper environment string (changed from control -> control-plane)

Tests

  • All unit tests pass
  • Deployed locally via docker-compose without issue in control-plane mode.
  • Successfully ran a sync locally in control-plane mode

@github-actions github-actions bot added area/platform issues related to the platform area/worker Related to worker labels Sep 27, 2022
Copy link
Contributor

@gosusnp gosusnp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels a lot cleaner.

@@ -163,7 +163,7 @@ public void onApplicationEvent(final ServiceReadyEvent event) {
log.info("Starting worker factory...");
workerFactory.start();

log.info("Application initialized (mode = {}).", workerPlane);
log.info("Application initialized.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we see the environment.getActiveNames somewhere? Otherwise, I think it could be worth logging for debugging purpose.

@jdpgrailsdev
Copy link
Contributor Author

@gosusnp @benmoriceau I realized that I also need to update the helm charts and Kube acceptance test charts. I will do that before merging.

@benmoriceau
Copy link
Contributor

@gosusnp @benmoriceau I realized that I also need to update the helm charts and Kube acceptance test charts. I will do that before merging.

FYI, the infra team is working on removing the kube acceptance test charts: https://airbytehq-team.slack.com/archives/C02TXT37Y3Y/p1664298082438109. One less chart to update!

@jdpgrailsdev
Copy link
Contributor Author

@gosusnp @benmoriceau PR has been updated with the fixed charts.

@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets September 28, 2022 13:42 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets September 28, 2022 14:05 Inactive
@jdpgrailsdev jdpgrailsdev merged commit 0ace816 into master Sep 28, 2022
@jdpgrailsdev jdpgrailsdev deleted the jonathan/worker-environment branch September 28, 2022 15:02
robbinhan pushed a commit to robbinhan/airbyte that referenced this pull request Sep 29, 2022
* Replace WorkerPlane with Micronaut environments

* Set proper environment for airbyte-workers
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
* Replace WorkerPlane with Micronaut environments

* Set proper environment for airbyte-workers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/platform issues related to the platform area/worker Related to worker kubernetes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants