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

Add local env runtime variables override for local testing #690

Closed
wants to merge 5 commits into from

Conversation

yuli-han
Copy link
Collaborator

@yuli-han yuli-han commented Feb 23, 2024

Summary

Currently Chronon set the runtime environment variables from several sources based on the below priority order:
- Environment variables existing already.
- Environment variables derived from args (like app_name)
- conf.metaData.modeToEnvMap for the mode (set on config)
- team environment per context and mode set on teams.json
- default team environment per context and mode set on teams.json
- Common Environment set in teams.json

The team environment per context mode set is a team level environment setting which defined in the json file teams.json. Team could put the environment settings(like EMR clusters and queue) based on their need, and these settings will be applied to all the jobs within the team folder.

However, sometimes our users may want to test their new job locally, the local tests may use different environment settings comparing with the prod run on airflow. Currently they need to add their job config into a different folder(like the test folder) other than the production folder(which will be parsed as context), and add context specific environment variables, but this may bring new complexity to the testing process since they still have to copy their changes to the production config to make it work in production environment.

This PR is trying to solve this problem by introducing a new arg env whose default value is dev. Users can define runtime environment variables for different environment in teams.json file, for example, for production environment and dev environment:

"production": {
    "backfill": {
        "EMR_CLUSTER": "backfill-shared-prod",
        "EMR_QUEUE": "backfill"
    },
    "upload": {
        "EMR_CLUSTER": "backfill-shared-prod",
        "EMR_QUEUE": "backfill"
    }
},
"dev": {
    "backfill": {
        "EMR_CLUSTER": "homes-shared-prod-ap",
        "EMR_QUEUE": "staging"
    }
},
  1. For production run on airflow, the job will be scheduled with --env=production, so the production environment will be used.
  2. For local testing, the default env is dev, so if the dev environment is provided in teams.json file, dev environment will be used, otherwise production environment will be used.

Why / Goal

Test Plan

  • Added Unit Tests
  • Covered by existing CI
  • Integration tested
  1. dev environment is provided in teams.json, env=production
    Local tests with
    python3 scripts/run.py --mode=backfill --conf=production/joins/zipline_test/test_online_join_small.v2 --ds=2024-02-20 --env=production
image
  1. dev environment is provided in teams.json, env=dev
    Local tests with
    python3 scripts/run.py --mode=backfill --conf=production/joins/zipline_test/test_online_join_small.v2 --ds=2024-02-20 --env=dev
image
  1. dev environment is provided in teams.json, env is not used
    Local tests with
    python3 scripts/run.py --mode=backfill --conf=production/joins/zipline_test/test_online_join_small.v2 --ds=2024-02-20
image
  1. dev environment is not provided in teams.json, env = dev
    Local tests with
    python3 scripts/run.py --mode=backfill --conf=production/joins/zipline_test/test_online_join_small.v2 --ds=2024-02-20 --env=dev
image

Checklist

  • Documentation update

Reviewers

@better365

@@ -326,7 +329,12 @@ def set_runtime_env(args):
environment["cli_args"]["CHRONON_DRIVER_JAR"] = args.chronon_jar
environment["cli_args"]["CHRONON_ONLINE_JAR"] = args.online_jar
environment["cli_args"]["CHRONON_ONLINE_CLASS"] = args.online_class
order = ["conf_env", "team_env", "default_env", "common_env", "cli_args"]
# If the job is running on airflow, ignore the dev team environment.
if 'AIRFLOW_CTX_EXECUTION_DATE' in os.environ:
Copy link
Collaborator

Choose a reason for hiding this comment

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

we want to leave the Airbnbism code within our data repo. So when we are ready to open source, it is more general here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Got it! If we cannot add the airflow related logic in chronon code, I am thinking on some other options:

  1. Ask users to add an extra test flag when they run the python job for local testing, like
    python3 ~/.local/bin/run.py --mode=backfill --conf=production/joins/zipline_test/test_online_join_small.v2 --ds=2022-07-02 --test
    And then in run.py file we can prioritize the dev_team_env if the test flag is set here.

  2. Ask users to put the compiled config files which they want to test into the test folder. And they can add environment variables as key test in the teams.json file so that run.py will automatically pick the test parameter. No code change is needed in run.py

  3. Nothing needed to be changed from the user side, but we add something in data repo code, like add --env=prod in airflow scheduling code. In run.py we try to get env from the command, if env is set to be prod, then use production env otherwise use dev_team_env. This will help the airbnb internal users but will introduce extra complexity for outside users(since they may not use airflow and they may need to add this env parameter)

Both option 1 and 2 needs users to change the way they do the testing. Wander do you have any other suggestions here? @better365

@yuli-han yuli-han changed the title [WIP]Add local env runtime variables override for local testing Add local env runtime variables override for local testing Feb 27, 2024
@@ -326,7 +336,7 @@ def set_runtime_env(args):
environment["cli_args"]["CHRONON_DRIVER_JAR"] = args.chronon_jar
environment["cli_args"]["CHRONON_ONLINE_JAR"] = args.online_jar
environment["cli_args"]["CHRONON_ONLINE_CLASS"] = args.online_class
order = ["conf_env", "team_env", "default_env", "common_env", "cli_args"]
order = ["conf_env", "team_env", "production_team_env", "default_env", "common_env", "cli_args"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we put production_team_env before team_env

Suggested change
order = ["conf_env", "team_env", "production_team_env", "default_env", "common_env", "cli_args"]
order = ["conf_env", "production_team_env""team_env", "default_env", "common_env", "cli_args"]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @better365 the PR has been moved to here #698
For this change if we put production_team_env over team_env, then even for local testing, as long as production environment is provided in team.json file it will always be picked right? We want to use dev environment if we set env=dev.

The logic is:
env=dev: production_team_env = production environment, team_env = dev team environment. and we want to use dev team environment.

@yuli-han
Copy link
Collaborator Author

PR has been moved to #698

@yuli-han yuli-han closed this Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants