Skip to content

Conversation

@oharan2
Copy link
Contributor

@oharan2 oharan2 commented Sep 11, 2023

Short description:
More details:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
Bug:

@redhat-qe-bot
Copy link
Contributor

Report bugs in Issues

The following are automatically added:

  • Add reviewers from OWNER file (in the root of the repository) under reviewers section.
  • Set PR size label.
  • New issue is created for the PR. (Closed when PR is merged/closed)
  • Run pre-commit if .pre-commit-config.yaml exists in the repo.

Available user actions:

  • To mark PR as WIP comment /wip to the PR, To remove it from the PR comment /wip cancel to the PR.
  • To block merging of PR comment /hold, To un-block merging of PR comment /hold cancel.
  • To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
    verified label removed on each new commit push.
  • To cherry pick a merged PR comment /cherry-pick <target branch to cherry-pick to> in the PR.
    • Multiple target branches can be cherry-picked, separated by spaces. (/cherry-pick branch1 branch2)
    • Cherry-pick will be started when PR is merged
  • To build and push container image command /build-and-push-container in the PR (tag will be the PR number).
  • To add a label by comment use /<label name>, to remove, use /<label name> cancel
Supported /retest check runs * /retest tox - Retest tox * /retest sonarqube - Retest sonarqube * /retest python-module-install - Retest python-module-install
Supported labels
  • hold
  • verified
  • wip
  • lgtm

"""
Args:
owner_references (list): List of objects depended on this object.
revision_object (int): indicates the revision of the state represented by Data.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rnetser @myakove I cannot verify this PR,
The changes I added are verified but the API docs mention the revision_object type is int, while line 39 is trying to interact with revision_object.res which doesn't match. What do you think?

See

Copy link
Collaborator

Choose a reason for hiding this comment

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

@oharan2 update revision_object description; it is not an int.
revision_object is object spec.

Copy link
Collaborator

Choose a reason for hiding this comment

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

see that revision (int64), required is missing

Copy link
Contributor Author

@oharan2 oharan2 Sep 11, 2023

Choose a reason for hiding this comment

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

I thought the revision_object and the revision parameter is referring the same, will update

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rnetser Do I need to validate the Int64 (as recently committed)?
many cases might pass a regular int and I suggested to maintain this requirement.

"""
Args:
owner_references (list): List of objects depended on this object.
revision_object (int): indicates the revision of the state represented by Data.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@oharan2 update revision_object description; it is not an int.
revision_object is object spec.

"""
Args:
owner_references (list): List of objects depended on this object.
revision_object (int): indicates the revision of the state represented by Data.
Copy link
Collaborator

Choose a reason for hiding this comment

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

see that revision (int64), required is missing

packaging = "^23.1"
python-simple-logger = "^1.0.6"
jinja2 = "^3.1.2"
numpy = "*"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do not edit this file directly, use poetry add

Args:
owner_references (list, optional): List of objects depended on this object.
revision_object (object, optional): the Data Object representing the state.
revision (int64): indicates the revision of the state represented by Data.
Copy link
Collaborator

Choose a reason for hiding this comment

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

You specify that the code expect int64 but in the code you convert it to int64
We need to get it from the user and only validate that the user was sent us int64

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants