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

Correct schema refresh logic #2785

Merged
merged 1 commit into from
Dec 6, 2022
Merged

Conversation

ssbarnea
Copy link
Member

@ssbarnea ssbarnea commented Dec 6, 2022

Please note that offline mode does disable schema update completely.

@erikgb
Copy link
Contributor

erikgb commented Dec 6, 2022

Answering #2729 (comment), since the issue is now locked. 😡

Thanks for the update @ssbarnea. Sorry for "raising my voice", but there have been numerous issues with this "thing" in the latest patch! releases of ansible-lint. I find it very annoying.

@MarkusTeufelberger
Copy link
Contributor

Please note that offline mode does disable schema completely.

What? Can't they be vendored into this repository for example?

@ssbarnea
Copy link
Member Author

ssbarnea commented Dec 6, 2022

Answering #2729 (comment), since the issue is now locked. 😡

Thanks for the update @ssbarnea. Sorry for "raising my voice", but there have been numerous issues with this "thing" in the latest patch! releases of ansible-lint. I find it very annoying.

It is ok. We all make mistakes and try to do better after. I will release this today. If you want to keep an eye on open pull requests, you might help us get them right from first attempt.

@ssbarnea
Copy link
Member Author

ssbarnea commented Dec 6, 2022

Please note that offline mode does disable schema completely.

What? Can't they be vendored into this repository for example?

They were vendored since day one, in fact since two days ago, ansible-lint is the official source of the schemas. We deprecated the https://github.com/ansible/schemas project by including it inside ansible-lint, to ease the maintenance.

We considered separated package but did not make sense, basically because inside a container, like https://github.com/ansible/creator-ee, we never update the packages. We wanted to ensure that someone that uses the linter on CI, github action or another pipeline, can benefit from using the latest schemas, without having to update the linter itself, especially because schemas changes are unlikely to cause any regressions.

@MarkusTeufelberger
Copy link
Contributor

Ah, I answered before you added the word "updates" there. This makes more sense of course.

@ssbarnea ssbarnea force-pushed the fix/schema-cache branch 2 times, most recently from 94e5c3e to b140fcb Compare December 6, 2022 17:50
- Ignore ConnectionResetError, reported on ansible#2729 (comment)
- Stop refresh process on first found error, to avoid delays when
  networking is not available
- Include etags for current schemas to avoid refreshing them
- Update schemas from linter repo too if they changed

Please note that offline mode does disable schema completely.
@ssbarnea ssbarnea merged commit f299d52 into ansible:main Dec 6, 2022
@ssbarnea ssbarnea deleted the fix/schema-cache branch December 6, 2022 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants