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

[Core] Bug fix for Tag Values representing booleans being loaded as boolean instead of strings #3482

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

JGSweets
Copy link
Contributor

@JGSweets JGSweets commented Apr 25, 2024

This PR updates jinja2 ray templates for gcp and aws when utilizing Tags.
Currently, boolean strings are inserted into the Ray YAML without being escaped. This fixes that by using the builtin tojson function for jinja2.

Otherwise, tags which had the values: false or true would fail to launch.

Tested (run the relevant ones):

  • Code formatting: bash format.sh
  • Any manual or new tests for this PR (please specify below)
    • AWS launch with tags which which include values of false and true set in the ~/.sky/config.yaml aws.instance_tags section
  • All smoke tests: pytest tests/test_smoke.py
  • Relevant individual smoke tests: pytest tests/test_smoke.py::test_fill_in_the_name
  • Backward compatibility tests: bash tests/backward_comaptibility_tests.sh

@JGSweets JGSweets changed the title [SERVE] Bug fix for Tag Values represents booleans being loaded as boolean instead of strings [SERVE] Bug fix for Tag Values representing booleans being loaded as boolean instead of strings Apr 25, 2024
@JGSweets JGSweets changed the title [SERVE] Bug fix for Tag Values representing booleans being loaded as boolean instead of strings [Core][SERVE] Bug fix for Tag Values representing booleans being loaded as boolean instead of strings Apr 26, 2024
@JGSweets JGSweets changed the title [Core][SERVE] Bug fix for Tag Values representing booleans being loaded as boolean instead of strings [Core][Serve] Bug fix for Tag Values representing booleans being loaded as boolean instead of strings Apr 26, 2024
Copy link
Collaborator

@Michaelvll Michaelvll left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this @JGSweets! Just tested with the following ~/.sky/config.yaml in master and got the following error, which is fixed in this PR.

aws:
  instance_tags:
    mykey1: "1"
    mykey2: "true"
    mykey3: "hello"
gcp:
  instance_tags:
    mykey1: "1"
    mykey2: "true"
    mykey3: "hello"
D 04-26 20:46:24 provisioner.py:183] botocore.exceptions.ParamValidationError: Parameter validation failed:
D 04-26 20:46:24 provisioner.py:183] Invalid type for parameter TagSpecifications[0].Tags[4].Value, value: 1, type: <class 'int'>, valid types: <class 'str'>
D 04-26 20:46:24 provisioner.py:183] Invalid type for parameter TagSpecifications[0].Tags[5].Value, value: True, type: <class 'bool'>, valid types: <class 'str'>

@Michaelvll Michaelvll changed the title [Core][Serve] Bug fix for Tag Values representing booleans being loaded as boolean instead of strings [Core] Bug fix for Tag Values representing booleans being loaded as boolean instead of strings Apr 26, 2024
@Michaelvll Michaelvll merged commit 32631ff into skypilot-org:master Apr 26, 2024
20 checks passed
JGSweets added a commit to JGSweets/skypilot that referenced this pull request May 1, 2024
…oolean instead of strings (skypilot-org#3482)

fix: string that represents boolean being loaded as boolean
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