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

Add incoming/indexer/storage information to V1 API's status endpoint #1078

Closed
wants to merge 1 commit into from

Conversation

unexceptable
Copy link
Contributor

Rebase of #995 with fixed for pep8.

@unexceptable
Copy link
Contributor Author

@chungg @tobias-urdin I've rebased this myself since it wasn't able to be cleanly done. Plus it looks like @leo-naeka might not be interested in carrying this anymore, so I might take over.

@unexceptable
Copy link
Contributor Author

the gnocchi.tests.indexer.sqlalchemy.test_migrations.ModelsMigrationsSync.test_models_sync failure is unrelated to this, and also failed on master.

So we should fix that and rebase this onto that. Meanwhile will also deal with the comments by @jd in the previous pull request.

@unexceptable unexceptable force-pushed the pyconfr2018 branch 2 times, most recently from caeeb37 to 9948f88 Compare November 3, 2020 22:23
@unexceptable
Copy link
Contributor Author

Getting tests to run locally is proving to be a bit of a pain, but py37-postgresql-file is passing locally, so I'm not sure what exactly us causing the 503 failures I'm seeing in travis-ci.

@jd
Copy link
Member

jd commented Nov 4, 2020

Some errors seem explicit:

    b'      "error": "\'FakeSwiftClient\' object has no attribute \'head_account\'"'

@unexceptable
Copy link
Contributor Author

Some errors seem explicit:

    b'      "error": "\'FakeSwiftClient\' object has no attribute \'head_account\'"'

Good catch! I've added that now to the FakeSwiftClient, but for now only returning an empty dict. Will have to think about what will be a more appropriate default test return value.

@unexceptable unexceptable force-pushed the pyconfr2018 branch 2 times, most recently from 7011278 to 65edf38 Compare November 5, 2020 05:39
@unexceptable
Copy link
Contributor Author

@jd Still have a few more of your comments from the other pull request to do, and need to add some more tests, but it's slowly starting to make sense to me. At least this code, and the project itself a little.

Although gabbi confuses me a little because finding the specific test it ran isn't a simple grep/text search due to the test name in repo appearing differently to the name in the failure. :( It might be worth renaming all the tests with _ rather than spaces to make them easier to search, or I wonder if gabbi has a setting for more verbose test names in failures to make that clearer.

@jd
Copy link
Member

jd commented Nov 5, 2020

@cdent might know about Gabbi. Usually, I didn't find those hard to find with a git grep and replacing the underscore.

@cdent
Copy link
Contributor

cdent commented Nov 5, 2020

replacing the underscore

That's what I tend to do.

I agree that it's kind of a pain, and an extra step, but there's a conflict between trying to make the yaml files somewhat readable (they are supposed to be as much a thing for humans as for computers) and the demands of unittest, stestr and other parts of the puzzle.

Certainly if one wanted to rename the name values in the gabbi file to use _ instead of space, that would be an option.

There's another complicating factor: Yesterday lbragstad noticed that when testtools was removed from gabbi, output and logging capture fixtures stopped working. That may make finding and dealing with failures a bit more messy. If that proves to be an issue, downgrading to gabbi<2.0.0 will fix it until cdent/gabbi#287 is resolved.

@unexceptable
Copy link
Contributor Author

@cdent I think for me gabbi is weird because I'm used to the python module.class.function path for tests, and gabbi fakes that. Maybe when a test fails it could also print out the relative file path location/test name? Or have a verbosity setting to enable that.

I found my way around it ultimately, but just felt a little weird for someone who tends to stick with more standard python unit testing.

@cdent
Copy link
Contributor

cdent commented Nov 6, 2020

used to the python module.class.function path for tests

The code that launches the tests is responsible for setting the "faked" name, see https://gabbi.readthedocs.io/en/latest/gabbi.html#gabbi.driver.build_tests

From that it's usually possible to make the test name point to the yaml file, or at least very close.

@tobias-urdin
Copy link
Contributor

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Mar 13, 2022

rebase

❌ Base branch update has failed

No oauth valid tokens
err-code: AF317

@tobias-urdin
Copy link
Contributor

@unexceptable Hello 👋 are you still working on this?

@tobias-urdin
Copy link
Contributor

Closing, feel free to reopen.

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

Successfully merging this pull request may close these issues.

5 participants