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

Get Pro status in managed LXD instance #652

Conversation

linostar
Copy link

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run tox?

@linostar linostar changed the base branch from main to feature/pro-sources September 11, 2024 11:28
Copy link

codecov bot commented Sep 11, 2024

Codecov Report

Attention: Patch coverage is 89.47368% with 2 lines in your changes missing coverage. Please review.

Please upload report for BASE (feature/pro-sources@5a98385). Learn more about missing BASE report.

Files with missing lines Patch % Lines
craft_providers/lxd/lxc.py 88.23% 2 Missing ⚠️
Additional details and impacted files
@@                  Coverage Diff                   @@
##             feature/pro-sources     #652   +/-   ##
======================================================
  Coverage                       ?   98.88%           
======================================================
  Files                          ?       54           
  Lines                          ?     4478           
  Branches                       ?        0           
======================================================
  Hits                           ?     4428           
  Misses                         ?       50           
  Partials                       ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@linostar linostar marked this pull request as ready for review September 13, 2024 12:50
@linostar
Copy link
Author

@canonical/starcraft-reviewers This is ready.

Copy link
Contributor

@lengau lengau left a comment

Choose a reason for hiding this comment

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

Could you add integration tests to this that check it with, say, Ubuntu Noble and Alma Linux 9?

@linostar
Copy link
Author

Could you add integration tests to this that check it with, say, Ubuntu Noble and Alma Linux 9?

@lengau We don't have spread tests in craft-providers. Perhaps we can do such tests in rockcraft once all those pro PRs are merged?

@lengau
Copy link
Contributor

lengau commented Sep 17, 2024

@linostar I'm requesting an integration test like the ones here: https://github.com/canonical/craft-providers/blob/main/tests/integration/lxd/test_lxc.py

It would create an instance and run these functions on that instance.

@linostar linostar force-pushed the ROCKS-1459/get-pro-status-in-lxd-instance branch 4 times, most recently from 7412faa to a489fa7 Compare September 18, 2024 08:02
@linostar linostar force-pushed the ROCKS-1459/get-pro-status-in-lxd-instance branch from a489fa7 to 773506b Compare September 18, 2024 08:20
Copy link
Contributor

@lengau lengau left a comment

Choose a reason for hiding this comment

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

Thanks @linostar , this looks good to me!

@lengau lengau merged commit 6425b68 into canonical:feature/pro-sources Sep 18, 2024
14 checks passed
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.

3 participants