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

Added Collection functionality to run cli cmd as collection #113

Closed
wants to merge 4 commits into from

Conversation

HariGS-DB
Copy link
Contributor

This change introduces the concept of collection in cli commands.

Background:
A cli cmd can be run as a current workspace installation command or as a collection.
When it is run as a collection, an Account client is created which can be used to run the cmd for all workspaces of a collection
When it is run as a non collection, a workspace client is created

Changes:

  • added is_collection bool to identify a cli cmd as eligible to run as collection
  • checking if collection_workspace_id is passed, if yes create a Account Client else create Workspace Client

@HariGS-DB HariGS-DB requested a review from nkvuong June 17, 2024 15:47
@HariGS-DB HariGS-DB requested a review from nfx as a code owner June 17, 2024 15:47
Copy link

github-actions bot commented Jun 17, 2024

✅ 12/12 passed, 2 skipped, 22s total

Running from acceptance #148

Copy link

codecov bot commented Jun 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.65%. Comparing base (7ab662f) to head (06d448e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #113      +/-   ##
==========================================
+ Coverage   79.00%   79.65%   +0.64%     
==========================================
  Files          14       14              
  Lines        1515     1524       +9     
  Branches      274      277       +3     
==========================================
+ Hits         1197     1214      +17     
+ Misses        230      224       -6     
+ Partials       88       86       -2     

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

@@ -30,6 +31,15 @@ def needs_workspace_client(self):
return False
return True

def run_as_collection(self) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

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

could you add a comment explaining how this would work, i.e. the command should have is_collection annotation & collection_workspace_id is not null

Copy link
Contributor

Choose a reason for hiding this comment

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

also, our function would always receive collection_workspace_id, the difference is that it might be null instead of an integer right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is my understanding. We mark a method as collection eligible by setting is_collection as True.
Now there are two ways to execute it , if the collection_workspace_id is passed, then create account context and iterate through all workspace context of the collection and run the associated command once for each workspace if collection_workspace_id is not passed, then run it as current installation cmd.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added comments

return False
sig = inspect.signature(self.fn)
for param in sig.parameters.values():
if param.name == "collection_workspace_id":
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we implement this on UCX only, for now?

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