Skip to content

[AL-1777] Update to Resource Tag test code #545

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Apr 19, 2022
Merged

[AL-1777] Update to Resource Tag test code #545

merged 3 commits into from
Apr 19, 2022

Conversation

jtsodapop
Copy link
Contributor

update to test_project. still leaving as skip as there is an issue wIth tag fetching. also updated resource creation code for better error messaging

@jtsodapop
update to test_project. still leaving as skip as there is an issue wi…
b8439c5 
…th tag fetching. also updated resource creation code for better error messaging
@jtsodapop jtsodapop requested review from manibox and msokoloff1 April 14, 2022 14:49
@jtsodapop
Copy link
Contributor Author

Unsure if code has recently been pushed out to change this, but I can only see Organization.resourceTags, and not resourceTag which is now causing organization.get_resource_tags to error out.

changing it to resourceTags consistently provides an internal server error. keeping as commented out until this can be addressed.

@manibox
Copy link
Contributor

manibox commented Apr 14, 2022

Unsure if code has recently been pushed out to change this, but I can only see Organization.resourceTags, and not resourceTag which is now causing organization.get_resource_tags to error out.

changing it to resourceTags consistently provides an internal server error. keeping as commented out until this can be addressed.

Hi! I believe if we revert a recent PR we pushed (for engineering) week, we can recover for now. https://github.com/Labelbox/intelligence/pull/8014

@manibox
Copy link
Contributor

manibox commented Apr 14, 2022

Just merged a revert which bring back the resourceTag endpoint. @jtsodapop lmk if anything else comes up

@jtsodapop
Copy link
Contributor Author

Just merged a revert which bring back the resourceTag endpoint. @jtsodapop lmk if anything else comes up

Thank you!!

manibox
manibox approved these changes Apr 19, 2022
View reviewed changes
Copy link
Contributor

@manibox manibox left a comment

Choose a reason for hiding this comment

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

lgtm!

labelbox/schema/organization.py
Comment on lines +154 to +156
if not all(params.values()):
raise ValueError(
f"tag must contain 'text' and 'color' keys. received: {tag}")
Copy link
Contributor

Choose a reason for hiding this comment

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

noice!

tests/integration/test_project.py
def test_update_project_resource_tags(client, rand_gen):

def delete_tag(tag_id: str):
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏽

@jtsodapop jtsodapop merged commit 3af9b59 into develop Apr 19, 2022
@jtsodapop jtsodapop deleted the jt/al-1777 branch April 19, 2022 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msokoloff1 msokoloff1 Awaiting requested review from msokoloff1

1 more reviewer

@manibox manibox manibox approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jtsodapop @manibox