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

Fix integration test on main, extract fallback CLI commands, add Azure fallback support and add sync fallback support #555

Merged
merged 9 commits into from
Sep 18, 2022

Conversation

parasj
Copy link
Contributor

@parasj parasj commented Sep 18, 2022

Fixes: #553

This PR makes several cleanups:

  • Merge UsageClient logic for exceptions and usage
  • Extract fallback command generation to another helper file
  • Implement fallback commands for sync
  • Implement fallback commands for Azure

Integration test rerun pending: https://github.com/skyplane-project/skyplane/actions/runs/3075774286

@parasj parasj changed the title Fix integration test on main and clean up native command fallback logic Fix integration test on main, extract fallback CLI commands, add Azure fallback support and add sync fallback support Sep 18, 2022
@parasj parasj marked this pull request as ready for review September 18, 2022 02:49
@ShishirPatil
Copy link
Member

Breaks skyplane cp for s3 - s3 transfer

Traceback (most recent call last):
  File "/home/ubuntu/miniconda/bin/skyplane", line 8, in <module>
    sys.exit(app())
  File "/home/ubuntu/miniconda/lib/python3.8/site-packages/typer/main.py", line 214, in __call__
    return get_command(self)(*args, **kwargs)
  File "/home/ubuntu/miniconda/lib/python3.8/site-packages/click/core.py", line 1126, in __call__
    return self.main(*args, **kwargs)
  File "/home/ubuntu/miniconda/lib/python3.8/site-packages/click/core.py", line 1051, in main
    rv = self.invoke(ctx)
  File "/home/ubuntu/miniconda/lib/python3.8/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/ubuntu/miniconda/lib/python3.8/site-packages/click/core.py", line 1393, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/ubuntu/miniconda/lib/python3.8/site-packages/click/core.py", line 752, in invoke
    return __callback(*args, **kwargs)
  File "/home/ubuntu/miniconda/lib/python3.8/site-packages/typer/main.py", line 500, in wrapper
    return callback(**use_params)  # type: ignore
  File "/home/ubuntu/skylark/skyplane/cli/cli.py", line 155, in cp
    src_client = ObjectStoreInterface.create(src_region_tag, bucket_src)
  File "/home/ubuntu/skylark/skyplane/obj_store/object_store_interface.py", line 109, in create
    raise ValueError(f"Invalid region_tag {region_tag} - could not create interface")
ValueError: Invalid region_tag s3:infer - could not create interface

Copy link
Contributor

@JasonDing0401 JasonDing0401 left a comment

Choose a reason for hiding this comment

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

The changes in the performance logging client look good to me!

Copy link
Member

@ShishirPatil ShishirPatil left a comment

Choose a reason for hiding this comment

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

As a design choice skyplane/cli/cli_impl/cp_replicate_fallback.py introduces isolation but increases the surface area. Do we want to stick to this long term?

@parasj
Copy link
Contributor Author

parasj commented Sep 18, 2022

As a design choice skyplane/cli/cli_impl/cp_replicate_fallback.py introduces isolation but increases the surface area. Do we want to stick to this long term?

We should eventually migrate off of this in the future, but I want to isolate this functionality so we can swap implementation as you implement improved on-prem functionality.

@parasj parasj merged commit 354dca8 into main Sep 18, 2022
@parasj parasj deleted the fix/553 branch September 18, 2022 20:46
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.

Skyplane integration tests are currently broken on main.
3 participants