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

Project: Image Services Gen2 #445

Merged
merged 7 commits into from
Aug 9, 2024
Merged

Project: Image Services Gen2 #445

merged 7 commits into from
Aug 9, 2024

Conversation

yec-akamai
Copy link
Contributor

@yec-akamai yec-akamai commented Jul 25, 2024

📝 Description

Project Image Gen2 LA

make testint TEST_SUITE=linode_client
make testint TEST_SUITE=image

yec-akamai and others added 2 commits July 2, 2024 12:55
* image gen2

* nit

* lint

* fix strenum import

* sort imports

* add int test

* address comments

* rename
@yec-akamai yec-akamai added the project for new projects in the changelog. label Jul 25, 2024
@yec-akamai yec-akamai marked this pull request as ready for review August 8, 2024 18:56
@yec-akamai yec-akamai requested a review from a team as a code owner August 8, 2024 18:56
@yec-akamai yec-akamai requested review from lgarber-akamai and ezilber-akamai and removed request for a team August 8, 2024 18:56
Copy link
Contributor

@ezilber-akamai ezilber-akamai left a comment

Choose a reason for hiding this comment

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

I got an error when running make testint TEST_SUITE=linode_client locally. It doesn't seem to be related to the changes though. I tried in a different branch and saw the same error.

_______________________________________________________________________________________ test_get_account_settings _______________________________________________________________________________________

test_linode_client = <linode_api4.linode_client.LinodeClient object at 0x104a0f100>

    def test_get_account_settings(test_linode_client):
        client = test_linode_client
        account_settings = client.account.settings()
    
        assert account_settings._populated == True
>       assert re.search(
            "'network_helper':True|False", str(account_settings._raw_json)
        )
E       assert None
E        +  where None = <function search at 0x102ac54c0>("'network_helper':True|False", "{'managed': True, 'network_helper': True, 'longview_subscription': 'longview-100', 'backups_enabled': True, 'object_storage': 'active'}")
E        +    where <function search at 0x102ac54c0> = re.search
E        +    and   "{'managed': True, 'network_helper': True, 'longview_subscription': 'longview-100', 'backups_enabled': True, 'object_storage': 'active'}" = str({'backups_enabled': True, 'longview_subscription': 'longview-100', 'managed': True, 'network_helper': True, ...})
E        +      where {'backups_enabled': True, 'longview_subscription': 'longview-100', 'managed': True, 'network_helper': True, ...} = AccountSettings: True._raw_json

@ezilber-akamai
Copy link
Contributor

I'm also running into a 404 when running make testint TEST_SUITE=image but this is likely because the test isn't being run in the correct region. Is there some additional setup I need to do to run the test against the right region?

@yec-akamai
Copy link
Contributor Author

I'm also running into a 404 when running make testint TEST_SUITE=image but this is likely because the test isn't being run in the correct region. Is there some additional setup I need to do to run the test against the right region?

Interesting, I've hardcoded the region to be us-east and it should work. Do you have the customer tag we mentioned in the thread?

@lgarber-akamai
Copy link
Contributor

lgarber-akamai commented Aug 8, 2024

@ezilber-akamai For the account settings test, it looks like the regex is looking for 'network_helper':True or just False instead of 'network_helper': True or 'network_helper': False. I'm guessing this is causing a failure on your end since your account doesn't seem to have any disabled settings.

I think the corrected pattern would look like this:

'network_helper':\s*(True|False)

If we don't want to include that fix in this PR since it's out of scope, it might be ticket-worthy 🙂

@yec-akamai
Copy link
Contributor Author

If we don't want to include that fix in this PR since it's out of sc

Good point! I included this fix in the PR. Also I changed a bit the get random region function to make sure we only get core site for testing. Some of the functions are not available in non-core sites.

@ezilber-akamai ezilber-akamai self-requested a review August 8, 2024 20:52
Copy link
Contributor

@ezilber-akamai ezilber-akamai left a comment

Choose a reason for hiding this comment

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

Tests passing locally. Nice work!

@ykim-1 ykim-1 self-requested a review August 9, 2024 15:28
@ykim-1
Copy link
Contributor

ykim-1 commented Aug 9, 2024

LGTM tests pass locally

@yec-akamai yec-akamai merged commit cea7eb2 into dev Aug 9, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project for new projects in the changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants