update SDK args

[alexxfan]

Issue link

RHOAIENG-7200

What changes have been made

What

Update the Cluster configuration in the SDK to make arguments clearer.

Currently the default config is as such:

cluster = Cluster(ClusterConfiguration(
    name='jobtest',
    namespace='default', # Update to your namespace
    num_workers=2,
    min_cpus=1,
    max_cpus=1,
    min_memory=4,
    max_memory=4,
    num_gpus=1,
    head_gpus=1,
    image="quay.io/project-codeflare/ray:latest-py39-cu118",
    write_to_file=False, # When enabled Ray Cluster yaml files are written to /HOME/.codeflare/resources 
    local_queue="local-queue-test" # Specify the local queue manually
)) 

How

Alter the following args:

• head_gpus -> num_head_gpus
• num_gpus -> num_worker_gpus
• min_cpus -> worker_cpu_requests
• max_cpus -> worker_cpu_limits
• min_memory -> worker_memory_requests
• max_memory -> worker_memory_limits

Verification steps

Setup

Notebook server ODH/RHOAI/Local

• Clone this repository with git clone https://github.com/project-codeflare/codeflare-sdk.git
• Checkout this PR's branch
• Run poetry build - install if needed (pip install poetry)
• Run pip install --force-reinstall dist/codeflare_sdk-0.0.0.dev0-py3-none-any.whl
• Restart your notebook kernel

Manual Testing

• Open any of the guided demo notebooks.
• Follow step by step using the old arguments, warnings should be thrown when ClusterConfiguration is run.
• Rerun the notebook using the new, suggested arguments, no errors should be given.

Checks

• I've made sure the tests are passing.
• Testing Strategy
  • Unit tests
  • Manual tests
  • Testing is not required for this change

[Ygnas]

Should we update the args in notebooks so there would be no deprecated warnings?

[Bobbins228]

@Ygnas users could be using the stack independently of notebooks so it's best to use these depreciation warnings

[Ygnas]

@Ygnas users could be using the stack independently of notebooks so it's best to use these depreciation warnings

@Ygnas users could be using the stack independently of notebooks so it's best to use these depreciation warnings

What I meant updating the notebooks would still show deprecated warnings for who use the old one, but would not have them if they they updated? Or I miss something here?

[Bobbins228]

@Ygnas I see what you mean now

We would update the notebooks after next release to avoid confusion for people using current release/older 👍

[ChughShilpa]

@Ygnas I see what you mean now

We would update the notebooks after next release to avoid confusion for people using current release/older 👍

Do we have a jira to track this work ?

[Bobbins228]

@ChughShilpa
Created an issue to track this https://issues.redhat.com/browse/RHOAIENG-7686

[Bobbins228]

@alexxfan
Can you install pre-commit and run pre-commit run --all-files which will auto format your code.
We have some instructions here

[alexxfan]

@Bobbins228 ran it there and pushed

[ChristianZaccaria]

A small bug and nitpicks, apart from that great work!!

[Bobbins228]

Hey Alex did you merge the main branch with the feature branch here?
I think you should undo that last merge commit and rebase your branch with the latest changes from main :)

[varshaprasad96]

@alexxfan Thanks for working on this! We would like to get this merged, but looks like it needs a rebase. Do you have some cycles to update the PR?

[alexxfan]

@varshaprasad96 that is updated now, thank you

[Bobbins228]

/lgtm

N/A

[varshaprasad96]

/lgtm
/approve

Just checked the branch out locally, and imports contain the args as expected! I assume the demo notebooks would be updated in follow-ups as a part of docs update.

[openshift-ci]

@varshaprasad96: changing LGTM is restricted to collaborators

In response to this:

/lgtm
/approve

Just checked the branch out locally, and imports contain the args as expected!

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[Bobbins228]

/lgtm
/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Bobbins228, varshaprasad96

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [Bobbins228,varshaprasad96]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Resolved

[dimakis]

@varshaprasad96 congrats on your first review for DW 😄 here's to many more.
Can I ask that you make sure there is an issue for updating the demos and that the details are updated? we also have some more demos here which we should also make sure are updated. 😄

[Bobbins228]

@dimakis I created an issue to track this already https://issues.redhat.com/browse/RHOAIENG-7686
I'll throw it into this sprint now and update it when #579 is merged so it can cover all of the new updated parameters.

[varshaprasad96]

Thanks @Bobbins228 for taking care of this. Do we also create an upstream GH issue to track the changes, or is JIRA sufficient?

[Bobbins228]

@varshaprasad96 The Jira board is public so that should be sufficient 👍