Skip to content

[Core][BugFix] Remove extra field _require_fuse in Resources.to_yaml_config()#3457

Merged
Michaelvll merged 1 commit intomasterfrom
fix-extra-required-fuse
Apr 22, 2024
Merged

[Core][BugFix] Remove extra field _require_fuse in Resources.to_yaml_config()#3457
Michaelvll merged 1 commit intomasterfrom
fix-extra-required-fuse

Conversation

@cblmemo
Copy link
Collaborator

@cblmemo cblmemo commented Apr 22, 2024

Fixes #3456

Tested (run the relevant ones):

  • Code formatting: bash format.sh
  • Any manual or new tests for this PR (please specify below)
[GCC 11.2.0] :: Anaconda, Inc. on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sky
>>> sky.Resources().to_yaml_config()
{'disk_size': 256}
  • 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

@Michaelvll
Copy link
Collaborator

Thanks for fixing this @cblmemo! Could we fix the schemas.py as well for those fields? It seems if it is included in the task yaml, and sent to the spot or serve controller, it will not be able to be loaded due to the schema check.

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.

Approving this PR and merging it first to unblock sky spot launch and sky serve up. We can fix the comments above in another PR.

@Michaelvll Michaelvll merged commit 1bdcd01 into master Apr 22, 2024
@Michaelvll Michaelvll deleted the fix-extra-required-fuse branch April 22, 2024 15:56
@cblmemo
Copy link
Collaborator Author

cblmemo commented Apr 23, 2024

Thanks for fixing this @cblmemo! Could we fix the schemas.py as well for those fields? It seems if it is included in the task yaml, and sent to the spot or serve controller, it will not be able to be loaded due to the schema check.

IIUC this is an internal usage and it should not be allowed..? And after this PR, it should not produce such field on the spot/serve controllers?

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.

[Core] Extra fields _require_fuse in Resources.to_yaml_config()

2 participants