Juju Lint

Merge juju-lint:update-snap into juju-lint:master

  1. Git
  2. lp:juju-lint
  3. update-snap
  4. Merge into master
Proposed by Gabriel Cocenza
Status: Merged
Approved by: Eric Chen
Approved revision: 57cd0951d0e5e453d304291142a69dabff5c560e
Merged at revision: 14ad2b7c55802329a6be651caa8a79773f0b3c2d
Proposed branch: juju-lint:update-snap
Merge into: juju-lint:master
Diff against target: 808 lines (+138/-279)
13 files modified
Makefile (+4/-1)
dev/null (+0/-199)
jujulint/check_spaces.py (+12/-7)
jujulint/cli.py (+7/-5)
jujulint/cloud.py (+7/-5)
jujulint/config.py (+2/-1)
jujulint/lint.py (+41/-27)
jujulint/logging.py (+2/-1)
snap/snapcraft.yaml (+3/-3)
tests/conftest.py (+3/-2)
tests/requirements.txt (+3/-1)
tests/test_jujulint.py (+28/-18)
tox.ini (+26/-9)
Reviewer Review Type Date Requested Status
Eric Chen Approve
Martin Kalcok (community) Approve
Review via email: mp+424672@code.launchpad.net

Commit message

update the snap

- added black to format the source code of the project and
  applied it into the code.
- added isort to order imports and applied it into the code.
- changed python version from 3.6(deprecated) to 3.8.
- changed base to core20.
- added /snap/bin to $PATH
- removed Pipfile, Pipfile.lock and .python-version that is not
  been used anymore.

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
Martin Kalcok (martin-kalcok) wrote :

I'm very much a fan of adding `black` and enforcing stricter rules. I have just few notes:

* I think that adding "black --check <targets>" to "lint" tox environment will help with CI integration
* Another useful tool is `isort` that manages format/ordering of python imports

If you want to check out example of other project that has these tools set up, you can check juju-verify [1].

Regarding the changes to snapcraft.yaml: Does juju-lint always depend on juju (snap)? If yes, maybe we should add it as `stage-snaps` [2]?

---
[1] https://github.com/canonical/juju-verify/blob/master/tox.ini
[2] https://snapcraft.io/docs/build-and-staging-dependencies#heading--package-names

review: Needs Fixing
Revision history for this message
Garrett Neugent (thogarre) wrote (last edit ):

This is not a blocker in anyway, but one other tool that is worth consideration is pre-commit. It can help enforce linting, unit tests, and other checks before engineers submit a new MR. Juju-backup-all has this implemented [0] if you think it's worth adding here. (Note that if you do consider adding this, you'll likely need to use v22.3.0 of black, not 21.8b0 as currently in juju-backup-all's config [1]).

[0] https://git.launchpad.net/juju-backup-all/tree/.pre-commit-config.yaml
[1] https://code.launchpad.net/~thogarre/juju-backup-all/+git/juju-backup-all/+merge/424590

Revision history for this message
Gabriel Cocenza (gabrielcocenza) wrote :

Hi Martin. Thanks for your review. I've added isort and applied it in the project.

Regarding adding juju snap into `stage-snaps` I guess it's not necessary. I don't see a huge gain of having the binaries of juju snap into juju-lint. Maybe we should consider using pythonlibjuju instead of making subprocess calls to deal with juju?

Revision history for this message
Gabriel Cocenza (gabrielcocenza) wrote :

Thanks for this suggestion Garret. I think it will be a very good thing for the project. I've added a bug[0] to add pre-commit in a future MR.

[0] https://bugs.launchpad.net/juju-lint/+bug/1978739

Revision history for this message
Robert Gildein (rgildein) wrote :

I briefly look at this and how three questions/suggestion:
- How do we use Pipenv here? Do you want to create a requirements.txt file? If
  so, what do you say to add `pipenv lock -> requirements.txt` to tox.ini to
  generate requirements from Pipfile? There is also simply way to check
  requirements in Pipfile.lock and requirements.txt
  `pipenv lock -r | diff requirements.txt -`
- If the above applies, we can use pipenv --dev to have testing requirements
  there.
- You can add `--cov-fail-under <current-percentage>` to ensure that each new
  function will need to have unittests.

Revision history for this message
Martin Kalcok (martin-kalcok) wrote :

Thanks for including my suggestions. Just two more nitpicks

* you have misaligned indentations in the pytest command in `unit` tox environment
* shouldn't the isort line length config (in tox.ini) match the line length of `black` which is 88?

review: Needs Fixing
Revision history for this message
Gabriel Cocenza (gabrielcocenza) wrote :

Thanks Martin. The problem in tox was mixed spaces and tabs. Regarding isort, you are right. I added the black profile [0] that will do the job.

Regarding the Pipfile(s), it looks like they are not been used (maybe just for those that use pipenv, so I removed then. The MR that added then it's this one [1].

[0] https://black.readthedocs.io/en/stable/guides/using_black_with_other_tools.html#profile
[1] https://code.launchpad.net/~ec0/juju-lint/+git/juju-lint/+merge/384517

Revision history for this message
Martin Kalcok (martin-kalcok) wrote :

LGTM, thanks

review: Approve
Revision history for this message
Eric Chen (eric-chen) wrote :

LGTM, please go ahead.

review: Approve
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision 14ad2b7c55802329a6be651caa8a79773f0b3c2d

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/.python-version b/.python-version
2deleted file mode 100644
3index a08ffae..0000000
4--- a/.python-version
5+++ /dev/null
6@@ -1 +0,0 @@
7-3.8.2
8diff --git a/Makefile b/Makefile
9index 70a2762..85be332 100644
10--- a/Makefile
11+++ b/Makefile
12@@ -1,5 +1,5 @@
13 lint:
14- tox -e py3-lint
15+ tox -e lintverbose
16
17 dch:
18 if ! which gbp > /dev/null; then sudo apt-get install -y git-buildpackage; fi
19@@ -11,6 +11,9 @@ deb-src:
20 test:
21 tox -e unit
22
23+format-code:
24+ tox -e format-code
25+
26 build:
27 snapcraft --use-lxd --debug
28
29diff --git a/Pipfile b/Pipfile
30deleted file mode 100644
31index d2acf80..0000000
32--- a/Pipfile
33+++ /dev/null
34@@ -1,17 +0,0 @@
35-[[source]]
36-url = "https://pypi.python.org/simple"
37-verify_ssl = true
38-name = "pypi"
39-
40-[packages]
41-attrs = ">=18.1.0"
42-colorlog = ">=4.1.0"
43-confuse = ">=1.1.0"
44-fabric2 = ">=2.5.0"
45-setuptools = ">=46.1.3"
46-PyYAML = ">=5.3.1"
47-
48-[dev-packages]
49-
50-[requires]
51-python_version = "3.6"
52diff --git a/Pipfile.lock b/Pipfile.lock
53deleted file mode 100644
54index c43f7b6..0000000
55--- a/Pipfile.lock
56+++ /dev/null
57@@ -1,199 +0,0 @@
58-{
59- "_meta": {
60- "hash": {
61- "sha256": "e1534fcda4a023c68f70df4e53b34a9bafcd9ec48c4f8d93e20dda25b3c43cd4"
62- },
63- "pipfile-spec": 6,
64- "requires": {
65- "python_version": "3.6"
66- },
67- "sources": [
68- {
69- "name": "pypi",
70- "url": "https://pypi.python.org/simple",
71- "verify_ssl": true
72- }
73- ]
74- },
75- "default": {
76- "attrs": {
77- "hashes": [
78- "sha256:08a96c641c3a74e44eb59afb61a24f2cb9f4d7188748e76ba4bb5edfa3cb7d1c",
79- "sha256:f7b7ce16570fe9965acd6d30101a28f62fb4a7f9e926b3bbc9b61f8b04247e72"
80- ],
81- "index": "pypi",
82- "version": "==19.3.0"
83- },
84- "bcrypt": {
85- "hashes": [
86- "sha256:0258f143f3de96b7c14f762c770f5fc56ccd72f8a1857a451c1cd9a655d9ac89",
87- "sha256:0b0069c752ec14172c5f78208f1863d7ad6755a6fae6fe76ec2c80d13be41e42",
88- "sha256:19a4b72a6ae5bb467fea018b825f0a7d917789bcfe893e53f15c92805d187294",
89- "sha256:5432dd7b34107ae8ed6c10a71b4397f1c853bd39a4d6ffa7e35f40584cffd161",
90- "sha256:6305557019906466fc42dbc53b46da004e72fd7a551c044a827e572c82191752",
91- "sha256:69361315039878c0680be456640f8705d76cb4a3a3fe1e057e0f261b74be4b31",
92- "sha256:6fe49a60b25b584e2f4ef175b29d3a83ba63b3a4df1b4c0605b826668d1b6be5",
93- "sha256:74a015102e877d0ccd02cdeaa18b32aa7273746914a6c5d0456dd442cb65b99c",
94- "sha256:763669a367869786bb4c8fcf731f4175775a5b43f070f50f46f0b59da45375d0",
95- "sha256:8b10acde4e1919d6015e1df86d4c217d3b5b01bb7744c36113ea43d529e1c3de",
96- "sha256:9fe92406c857409b70a38729dbdf6578caf9228de0aef5bc44f859ffe971a39e",
97- "sha256:a190f2a5dbbdbff4b74e3103cef44344bc30e61255beb27310e2aec407766052",
98- "sha256:a595c12c618119255c90deb4b046e1ca3bcfad64667c43d1166f2b04bc72db09",
99- "sha256:c9457fa5c121e94a58d6505cadca8bed1c64444b83b3204928a866ca2e599105",
100- "sha256:cb93f6b2ab0f6853550b74e051d297c27a638719753eb9ff66d1e4072be67133",
101- "sha256:ce4e4f0deb51d38b1611a27f330426154f2980e66582dc5f438aad38b5f24fc1",
102- "sha256:d7bdc26475679dd073ba0ed2766445bb5b20ca4793ca0db32b399dccc6bc84b7",
103- "sha256:ff032765bb8716d9387fd5376d987a937254b0619eff0972779515b5c98820bc"
104- ],
105- "version": "==3.1.7"
106- },
107- "cffi": {
108- "hashes": [
109- "sha256:001bf3242a1bb04d985d63e138230802c6c8d4db3668fb545fb5005ddf5bb5ff",
110- "sha256:00789914be39dffba161cfc5be31b55775de5ba2235fe49aa28c148236c4e06b",
111- "sha256:028a579fc9aed3af38f4892bdcc7390508adabc30c6af4a6e4f611b0c680e6ac",
112- "sha256:14491a910663bf9f13ddf2bc8f60562d6bc5315c1f09c704937ef17293fb85b0",
113- "sha256:1cae98a7054b5c9391eb3249b86e0e99ab1e02bb0cc0575da191aedadbdf4384",
114- "sha256:2089ed025da3919d2e75a4d963d008330c96751127dd6f73c8dc0c65041b4c26",
115- "sha256:2d384f4a127a15ba701207f7639d94106693b6cd64173d6c8988e2c25f3ac2b6",
116- "sha256:337d448e5a725bba2d8293c48d9353fc68d0e9e4088d62a9571def317797522b",
117- "sha256:399aed636c7d3749bbed55bc907c3288cb43c65c4389964ad5ff849b6370603e",
118- "sha256:3b911c2dbd4f423b4c4fcca138cadde747abdb20d196c4a48708b8a2d32b16dd",
119- "sha256:3d311bcc4a41408cf5854f06ef2c5cab88f9fded37a3b95936c9879c1640d4c2",
120- "sha256:62ae9af2d069ea2698bf536dcfe1e4eed9090211dbaafeeedf5cb6c41b352f66",
121- "sha256:66e41db66b47d0d8672d8ed2708ba91b2f2524ece3dee48b5dfb36be8c2f21dc",
122- "sha256:675686925a9fb403edba0114db74e741d8181683dcf216be697d208857e04ca8",
123- "sha256:7e63cbcf2429a8dbfe48dcc2322d5f2220b77b2e17b7ba023d6166d84655da55",
124- "sha256:8a6c688fefb4e1cd56feb6c511984a6c4f7ec7d2a1ff31a10254f3c817054ae4",
125- "sha256:8c0ffc886aea5df6a1762d0019e9cb05f825d0eec1f520c51be9d198701daee5",
126- "sha256:95cd16d3dee553f882540c1ffe331d085c9e629499ceadfbda4d4fde635f4b7d",
127- "sha256:99f748a7e71ff382613b4e1acc0ac83bf7ad167fb3802e35e90d9763daba4d78",
128- "sha256:b8c78301cefcf5fd914aad35d3c04c2b21ce8629b5e4f4e45ae6812e461910fa",
129- "sha256:c420917b188a5582a56d8b93bdd8e0f6eca08c84ff623a4c16e809152cd35793",
130- "sha256:c43866529f2f06fe0edc6246eb4faa34f03fe88b64a0a9a942561c8e22f4b71f",
131- "sha256:cab50b8c2250b46fe738c77dbd25ce017d5e6fb35d3407606e7a4180656a5a6a",
132- "sha256:cef128cb4d5e0b3493f058f10ce32365972c554572ff821e175dbc6f8ff6924f",
133- "sha256:cf16e3cf6c0a5fdd9bc10c21687e19d29ad1fe863372b5543deaec1039581a30",
134- "sha256:e56c744aa6ff427a607763346e4170629caf7e48ead6921745986db3692f987f",
135- "sha256:e577934fc5f8779c554639376beeaa5657d54349096ef24abe8c74c5d9c117c3",
136- "sha256:f2b0fa0c01d8a0c7483afd9f31d7ecf2d71760ca24499c8697aeb5ca37dc090c"
137- ],
138- "version": "==1.14.0"
139- },
140- "colorlog": {
141- "hashes": [
142- "sha256:30aaef5ab2a1873dec5da38fd6ba568fa761c9fa10b40241027fa3edea47f3d2",
143- "sha256:732c191ebbe9a353ec160d043d02c64ddef9028de8caae4cfa8bd49b6afed53e"
144- ],
145- "index": "pypi",
146- "version": "==4.1.0"
147- },
148- "confuse": {
149- "hashes": [
150- "sha256:adc1979ea6f4c0dd3d6fe06020c189843a649082ab8f6fb54db16f4ac5e5e1da"
151- ],
152- "index": "pypi",
153- "version": "==1.1.0"
154- },
155- "cryptography": {
156- "hashes": [
157- "sha256:091d31c42f444c6f519485ed528d8b451d1a0c7bf30e8ca583a0cac44b8a0df6",
158- "sha256:18452582a3c85b96014b45686af264563e3e5d99d226589f057ace56196ec78b",
159- "sha256:1dfa985f62b137909496e7fc182dac687206d8d089dd03eaeb28ae16eec8e7d5",
160- "sha256:1e4014639d3d73fbc5ceff206049c5a9a849cefd106a49fa7aaaa25cc0ce35cf",
161- "sha256:22e91636a51170df0ae4dcbd250d318fd28c9f491c4e50b625a49964b24fe46e",
162- "sha256:3b3eba865ea2754738616f87292b7f29448aec342a7c720956f8083d252bf28b",
163- "sha256:651448cd2e3a6bc2bb76c3663785133c40d5e1a8c1a9c5429e4354201c6024ae",
164- "sha256:726086c17f94747cedbee6efa77e99ae170caebeb1116353c6cf0ab67ea6829b",
165- "sha256:844a76bc04472e5135b909da6aed84360f522ff5dfa47f93e3dd2a0b84a89fa0",
166- "sha256:88c881dd5a147e08d1bdcf2315c04972381d026cdb803325c03fe2b4a8ed858b",
167- "sha256:96c080ae7118c10fcbe6229ab43eb8b090fccd31a09ef55f83f690d1ef619a1d",
168- "sha256:a0c30272fb4ddda5f5ffc1089d7405b7a71b0b0f51993cb4e5dbb4590b2fc229",
169- "sha256:bb1f0281887d89617b4c68e8db9a2c42b9efebf2702a3c5bf70599421a8623e3",
170- "sha256:c447cf087cf2dbddc1add6987bbe2f767ed5317adb2d08af940db517dd704365",
171- "sha256:c4fd17d92e9d55b84707f4fd09992081ba872d1a0c610c109c18e062e06a2e55",
172- "sha256:d0d5aeaedd29be304848f1c5059074a740fa9f6f26b84c5b63e8b29e73dfc270",
173- "sha256:daf54a4b07d67ad437ff239c8a4080cfd1cc7213df57d33c97de7b4738048d5e",
174- "sha256:e993468c859d084d5579e2ebee101de8f5a27ce8e2159959b6673b418fd8c785",
175- "sha256:f118a95c7480f5be0df8afeb9a11bd199aa20afab7a96bcf20409b411a3a85f0"
176- ],
177- "version": "==2.9.2"
178- },
179- "fabric2": {
180- "hashes": [
181- "sha256:29edd7848420df589a49743394a0ae6874ccb6a9fe6413c0076d42cc290dcad6",
182- "sha256:8838d9641fd4e95bfc2568aa16fc683a600de860ac52a1dc9675a4db3c6cef7c"
183- ],
184- "index": "pypi",
185- "version": "==2.5.0"
186- },
187- "invoke": {
188- "hashes": [
189- "sha256:87b3ef9d72a1667e104f89b159eaf8a514dbf2f3576885b2bbdefe74c3fb2132",
190- "sha256:93e12876d88130c8e0d7fd6618dd5387d6b36da55ad541481dfa5e001656f134",
191- "sha256:de3f23bfe669e3db1085789fd859eb8ca8e0c5d9c20811e2407fa042e8a5e15d"
192- ],
193- "version": "==1.4.1"
194- },
195- "paramiko": {
196- "hashes": [
197- "sha256:920492895db8013f6cc0179293147f830b8c7b21fdfc839b6bad760c27459d9f",
198- "sha256:9c980875fa4d2cb751604664e9a2d0f69096643f5be4db1b99599fe114a97b2f"
199- ],
200- "version": "==2.7.1"
201- },
202- "pycparser": {
203- "hashes": [
204- "sha256:2d475327684562c3a96cc71adf7dc8c4f0565175cf86b6d7a404ff4c771f15f0",
205- "sha256:7582ad22678f0fcd81102833f60ef8d0e57288b6b5fb00323d101be910e35705"
206- ],
207- "version": "==2.20"
208- },
209- "pynacl": {
210- "hashes": [
211- "sha256:06cbb4d9b2c4bd3c8dc0d267416aaed79906e7b33f114ddbf0911969794b1cc4",
212- "sha256:11335f09060af52c97137d4ac54285bcb7df0cef29014a1a4efe64ac065434c4",
213- "sha256:2fe0fc5a2480361dcaf4e6e7cea00e078fcda07ba45f811b167e3f99e8cff574",
214- "sha256:30f9b96db44e09b3304f9ea95079b1b7316b2b4f3744fe3aaecccd95d547063d",
215- "sha256:511d269ee845037b95c9781aa702f90ccc36036f95d0f31373a6a79bd8242e25",
216- "sha256:537a7ccbea22905a0ab36ea58577b39d1fa9b1884869d173b5cf111f006f689f",
217- "sha256:54e9a2c849c742006516ad56a88f5c74bf2ce92c9f67435187c3c5953b346505",
218- "sha256:757250ddb3bff1eecd7e41e65f7f833a8405fede0194319f87899690624f2122",
219- "sha256:7757ae33dae81c300487591c68790dfb5145c7d03324000433d9a2c141f82af7",
220- "sha256:7c6092102219f59ff29788860ccb021e80fffd953920c4a8653889c029b2d420",
221- "sha256:8122ba5f2a2169ca5da936b2e5a511740ffb73979381b4229d9188f6dcb22f1f",
222- "sha256:9c4a7ea4fb81536c1b1f5cc44d54a296f96ae78c1ebd2311bd0b60be45a48d96",
223- "sha256:cd401ccbc2a249a47a3a1724c2918fcd04be1f7b54eb2a5a71ff915db0ac51c6",
224- "sha256:d452a6746f0a7e11121e64625109bc4468fc3100452817001dbe018bb8b08514",
225- "sha256:ea6841bc3a76fa4942ce00f3bda7d436fda21e2d91602b9e21b7ca9ecab8f3ff",
226- "sha256:f8851ab9041756003119368c1e6cd0b9c631f46d686b3904b18c0139f4419f80"
227- ],
228- "version": "==1.4.0"
229- },
230- "pyyaml": {
231- "hashes": [
232- "sha256:06a0d7ba600ce0b2d2fe2e78453a470b5a6e000a985dd4a4e54e436cc36b0e97",
233- "sha256:240097ff019d7c70a4922b6869d8a86407758333f02203e0fc6ff79c5dcede76",
234- "sha256:4f4b913ca1a7319b33cfb1369e91e50354d6f07a135f3b901aca02aa95940bd2",
235- "sha256:69f00dca373f240f842b2931fb2c7e14ddbacd1397d57157a9b005a6a9942648",
236- "sha256:73f099454b799e05e5ab51423c7bcf361c58d3206fa7b0d555426b1f4d9a3eaf",
237- "sha256:74809a57b329d6cc0fdccee6318f44b9b8649961fa73144a98735b0aaf029f1f",
238- "sha256:7739fc0fa8205b3ee8808aea45e968bc90082c10aef6ea95e855e10abf4a37b2",
239- "sha256:95f71d2af0ff4227885f7a6605c37fd53d3a106fcab511b8860ecca9fcf400ee",
240- "sha256:b8eac752c5e14d3eca0e6dd9199cd627518cb5ec06add0de9d32baeee6fe645d",
241- "sha256:cc8955cfbfc7a115fa81d85284ee61147059a753344bc51098f3ccd69b0d7e0c",
242- "sha256:d13155f591e6fcc1ec3b30685d50bf0711574e2c0dfffd7644babf8b5102ca1a"
243- ],
244- "index": "pypi",
245- "version": "==5.3.1"
246- },
247- "six": {
248- "hashes": [
249- "sha256:30639c035cdb23534cd4aa2dd52c3bf48f06e5f4a941509c8bafd8ce11080259",
250- "sha256:8b74bedcbbbaca38ff6d7491d76f2b06b3592611af620f8426e82dddb04a5ced"
251- ],
252- "version": "==1.15.0"
253- }
254- },
255- "develop": {}
256-}
257diff --git a/jujulint/check_spaces.py b/jujulint/check_spaces.py
258index 47c0c63..17b8d8c 100644
259--- a/jujulint/check_spaces.py
260+++ b/jujulint/check_spaces.py
261@@ -20,7 +20,9 @@ class Relation:
262 While Juju does define separate provider and requirer roles, we'll ignore
263 those here.
264 """
265- return set([self.endpoint1, self.endpoint2]) == set([other.endpoint1, other.endpoint2])
266+ return set([self.endpoint1, self.endpoint2]) == set(
267+ [other.endpoint1, other.endpoint2]
268+ )
269
270 @property
271 def endpoints(self):
272@@ -45,7 +47,8 @@ class SpaceMismatch:
273 def __str__(self):
274 """Stringify the object."""
275 return "SpaceMismatch({} (space {}) != {} (space {}))".format(
276- self.endpoint1, self.space1, self.endpoint2, self.space2)
277+ self.endpoint1, self.space1, self.endpoint2, self.space2
278+ )
279
280 @property
281 def relation(self):
282@@ -54,8 +57,8 @@ class SpaceMismatch:
283
284 def get_charm_relation(self, app_to_charm_map):
285 """Return a relation object, mapping applications to charms."""
286- app1, endpoint1 = self.endpoint1.split(':')
287- app2, endpoint2 = self.endpoint2.split(':')
288+ app1, endpoint1 = self.endpoint1.split(":")
289+ app2, endpoint2 = self.endpoint2.split(":")
290 charm1 = app_to_charm_map[app1]
291 charm2 = app_to_charm_map[app2]
292 return Relation(":".join([charm1, endpoint1]), ":".join([charm2, endpoint2]))
293@@ -92,7 +95,9 @@ def find_space_mismatches(parsed_yaml, debug=False):
294 space1 = get_relation_space(relation.endpoint1, app_spaces)
295 space2 = get_relation_space(relation.endpoint2, app_spaces)
296 if space1 != space2:
297- mismatch = SpaceMismatch(relation.endpoint1, space1, relation.endpoint2, space2)
298+ mismatch = SpaceMismatch(
299+ relation.endpoint1, space1, relation.endpoint2, space2
300+ )
301 mismatches.append(mismatch)
302
303 if debug:
304@@ -112,7 +117,7 @@ def get_application_spaces(application_list, parsed_yaml):
305 app_spaces = {}
306 for app in application_list:
307 app_spaces[app] = {}
308- bindings = parsed_yaml["applications"][app]['bindings']
309+ bindings = parsed_yaml["applications"][app]["bindings"]
310 for name, value in bindings.items():
311 app_spaces[app][name] = value
312 return app_spaces
313@@ -129,7 +134,7 @@ def get_application_relations(parsed_yaml):
314
315 def get_relation_space(endpoint, app_spaces):
316 """Get space for specified app and service."""
317- app, service = endpoint.split(':')
318+ app, service = endpoint.split(":")
319 if app in app_spaces:
320 if service not in app_spaces[app]:
321 return app_spaces[app][""]
322diff --git a/jujulint/cli.py b/jujulint/cli.py
323index af5190a..2d76832 100755
324--- a/jujulint/cli.py
325+++ b/jujulint/cli.py
326@@ -17,15 +17,17 @@
327 # You should have received a copy of the GNU General Public License
328 # along with this program. If not, see <http://www.gnu.org/licenses/>.
329 """Main entrypoint for the juju-lint CLI."""
330-from jujulint.config import Config
331-from jujulint.lint import Linter
332-from jujulint.logging import Logger
333-from jujulint.openstack import OpenStack
334 import logging
335 import os.path
336+import sys
337+
338 import pkg_resources
339 import yaml
340-import sys
341+
342+from jujulint.config import Config
343+from jujulint.lint import Linter
344+from jujulint.logging import Logger
345+from jujulint.openstack import OpenStack
346
347
348 class Cli:
349diff --git a/jujulint/cloud.py b/jujulint/cloud.py
350index b3cde24..3382e3b 100644
351--- a/jujulint/cloud.py
352+++ b/jujulint/cloud.py
353@@ -33,13 +33,15 @@ Todo:
354 * Add function to run command on a unit, via fabric and jump host if configured
355
356 """
357-from fabric2 import Connection, Config
358-from paramiko.ssh_exception import SSHException
359-from jujulint.logging import Logger
360-from jujulint.lint import Linter
361-from subprocess import check_output
362 import socket
363+from subprocess import check_output
364+
365 import yaml
366+from fabric2 import Config, Connection
367+from paramiko.ssh_exception import SSHException
368+
369+from jujulint.lint import Linter
370+from jujulint.logging import Logger
371
372
373 class Cloud:
374diff --git a/jujulint/config.py b/jujulint/config.py
375index 1173187..d87677a 100644
376--- a/jujulint/config.py
377+++ b/jujulint/config.py
378@@ -18,9 +18,10 @@
379 # along with this program. If not, see <http://www.gnu.org/licenses/>.
380 """Config handling routines."""
381
382-from confuse import Configuration
383 from argparse import ArgumentParser
384
385+from confuse import Configuration
386+
387
388 class Config(Configuration):
389 """Helper class for holding parsed config, extending confuse's BaseConfiguraion class."""
390diff --git a/jujulint/lint.py b/jujulint/lint.py
391index 404e8b8..e908b7a 100755
392--- a/jujulint/lint.py
393+++ b/jujulint/lint.py
394@@ -19,24 +19,23 @@
395 # along with this program. If not, see <http://www.gnu.org/licenses/>.
396 """Lint operations and rule processing engine."""
397 import collections
398-from datetime import datetime, timezone
399 import json
400 import logging
401 import os.path
402 import pprint
403 import re
404 import traceback
405+from datetime import datetime, timezone
406
407-import yaml
408-
409-from attr import attrs, attrib
410 import attr
411 import dateutil.parser
412+import yaml
413+from attr import attrib, attrs
414 from dateutil import relativedelta
415
416 import jujulint.util as utils
417+from jujulint.check_spaces import Relation, find_space_mismatches
418 from jujulint.logging import Logger
419-from jujulint.check_spaces import find_space_mismatches, Relation
420
421 VALID_CONFIG_CHECKS = ("isset", "eq", "neq", "gte")
422
423@@ -195,7 +194,7 @@ class Linter:
424 if val[-1].lower() == val[-1]:
425 quotient = 1000
426
427- conv = {"g": quotient ** 3, "m": quotient ** 2, "k": quotient}
428+ conv = {"g": quotient**3, "m": quotient**2, "k": quotient}
429
430 return _int * conv[val[-1].lower()]
431
432@@ -684,21 +683,39 @@ class Linter:
433 enforce_endpoints = space_checks.get("enforce endpoints", [])
434 enforce_relations = [
435 Relation(*relation)
436- for relation in space_checks.get("enforce relations", [])]
437+ for relation in space_checks.get("enforce relations", [])
438+ ]
439 ignore_endpoints = space_checks.get("ignore endpoints", [])
440 ignore_relations = [
441- Relation(*relation)
442- for relation in space_checks.get("ignore relations", [])]
443+ Relation(*relation) for relation in space_checks.get("ignore relations", [])
444+ ]
445
446 mismatches = find_space_mismatches(parsed_yaml)
447 for mismatch in mismatches:
448 try:
449- self._handle_space_mismatch(mismatch, enforce_endpoints, enforce_relations, ignore_endpoints, ignore_relations)
450+ self._handle_space_mismatch(
451+ mismatch,
452+ enforce_endpoints,
453+ enforce_relations,
454+ ignore_endpoints,
455+ ignore_relations,
456+ )
457 except Exception:
458 # FOR NOW: super quick and dirty
459- print('Exception caught during space check; please check space by hand. {}'.format(traceback.format_exc()))
460+ print(
461+ "Exception caught during space check; please check space by hand. {}".format(
462+ traceback.format_exc()
463+ )
464+ )
465
466- def _handle_space_mismatch(self, mismatch, enforce_endpoints, enforce_relations, ignore_endpoints, ignore_relations):
467+ def _handle_space_mismatch(
468+ self,
469+ mismatch,
470+ enforce_endpoints,
471+ enforce_relations,
472+ ignore_endpoints,
473+ ignore_relations,
474+ ):
475 # By default: treat mismatches as warnings.
476 # If we have a matching enforcement rule, treat as an error.
477 # If we have a matching ignore rule, do not warn.
478@@ -722,12 +739,14 @@ class Linter:
479
480 message = "Space binding mismatch: {}".format(mismatch)
481 if error:
482- self.handle_error({
483- "id": "space-binding-mismatch",
484- "tags": ["mismatch", "space", "binding"],
485- "description": "Unhandled space binding mismatch",
486- "message": message,
487- })
488+ self.handle_error(
489+ {
490+ "id": "space-binding-mismatch",
491+ "tags": ["mismatch", "space", "binding"],
492+ "description": "Unhandled space binding mismatch",
493+ "message": message,
494+ }
495+ )
496 elif warning:
497 # DEFAULT: not a critical error, so just warn
498 self._log_with_header(message, level=logging.WARN)
499@@ -1041,7 +1060,7 @@ class Linter:
500 offer_overlay = True
501 if parsed_yaml is None or not offer_overlay:
502 parsed_yaml = doc
503- return(parsed_yaml)
504+ return parsed_yaml
505
506 def lint_yaml_string(self, yaml_string):
507 """Lint provided YAML string."""
508@@ -1086,15 +1105,10 @@ class Linter:
509
510 if "relations" in parsed_yaml:
511 # "bindings" *should* be in exported bundles, *unless* no custom bindings exist,
512- # in which case "juju export-bundle" omits them.
513+ # in which case "juju export-bundle" omits them. See LP#1949883.
514 if "bindings" in list(parsed_yaml[applications].values())[0]:
515- #try:
516- self.check_spaces(parsed_yaml)
517- #except Exception as e:
518- # self._log_with_header(
519- # "Encountered error while checking spaces: {}".format(e),
520- # level=logging.WARN
521- # )
522+ self.check_spaces(parsed_yaml)
523+
524 else:
525 self._log_with_header(
526 "Relations detected but custom bindings not found; "
527diff --git a/jujulint/logging.py b/jujulint/logging.py
528index e3c42f8..1d1586d 100644
529--- a/jujulint/logging.py
530+++ b/jujulint/logging.py
531@@ -17,10 +17,11 @@
532 # You should have received a copy of the GNU General Public License
533 # along with this program. If not, see <http://www.gnu.org/licenses/>.
534 """Logging helper functions."""
535-import colorlog
536 import logging
537 import sys
538
539+import colorlog
540+
541
542 class Logger:
543 """Helper class for logging."""
544diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml
545index 9f84064..555e9af 100644
546--- a/snap/snapcraft.yaml
547+++ b/snap/snapcraft.yaml
548@@ -1,6 +1,6 @@
549 ---
550 name: juju-lint
551-base: core18
552+base: core20
553 summary: Linter for Juju models to compare deployments with configurable policy
554 adopt-info: juju-lint
555 description: |
556@@ -13,8 +13,8 @@ apps:
557 juju-lint:
558 command: bin/juju-lint
559 environment:
560- PATH: "/snap/juju-lint/current/bin:/snap/juju-lint/current/usr/bin:/bin:/usr/bin:"
561- PYTHONPATH: $SNAP/usr/lib/python3.6/site-packages:$SNAP/usr/lib/python3.6/dist-packages:$PYTHONPATH
562+ PATH: "/snap/juju-lint/current/bin:/snap/juju-lint/current/usr/bin:/bin:/usr/bin:/snap/bin"
563+ PYTHONPATH: $SNAP/usr/lib/python3.8/site-packages:$SNAP/usr/lib/python3.8/dist-packages:$PYTHONPATH
564 parts:
565 juju-lint:
566 plugin: python
567diff --git a/tests/conftest.py b/tests/conftest.py
568index fc7093e..55ac1ac 100644
569--- a/tests/conftest.py
570+++ b/tests/conftest.py
571@@ -8,11 +8,12 @@
572
573 """Test fixtures for juju-lint tool."""
574
575-import mock
576 import os
577-import pytest
578 import sys
579
580+import mock
581+import pytest
582+
583 # bring in top level library to path
584 test_path = os.path.dirname(os.path.abspath(__file__))
585 sys.path.insert(0, test_path + "/../")
586diff --git a/tests/requirements.txt b/tests/requirements.txt
587index b40fd2a..2575eb3 100644
588--- a/tests/requirements.txt
589+++ b/tests/requirements.txt
590@@ -1,8 +1,10 @@
591 # Module requirements
592+black
593 flake8
594 flake8-colors
595 flake8-docstrings
596 flake8-html
597+isort
598 mock
599 pep8-naming
600 pycodestyle
601@@ -10,4 +12,4 @@ pyflakes
602 pytest
603 pytest-cov
604 pytest-html
605-pytest-mock
606\ No newline at end of file
607+pytest-mock
608diff --git a/tests/test_jujulint.py b/tests/test_jujulint.py
609index c4c51f3..cff1d6b 100644
610--- a/tests/test_jujulint.py
611+++ b/tests/test_jujulint.py
612@@ -1,8 +1,8 @@
613 #!/usr/bin/python3
614 """Tests for jujulint."""
615+import logging
616 from datetime import datetime, timezone
617 from unittest import mock
618-import logging
619
620 import pytest
621
622@@ -106,7 +106,7 @@ class TestLinter:
623 "test-app-4": {"charm": "local:SERIES/TEST-CHARM12"},
624 "test-app-5": {"charm": "local:TEST-CHARM12"},
625 "test-app-6": {"charm": "cs:~TEST-CHARMERS/TEST-CHARM12-123"},
626- "test-app-7": {"charm": "ch:amd64/bionic/TEST-CHARM12-123"}
627+ "test-app-7": {"charm": "ch:amd64/bionic/TEST-CHARM12-123"},
628 }
629 linter.map_charms(applications)
630 for charm in linter.model.charms:
631@@ -524,9 +524,7 @@ applications:
632
633 def test_config_eq_no_suffix_check_all(self, linter, juju_status):
634 """Test the config condition 'eq'. when no suffix all should be checked."""
635- linter.lint_rules["config"] = {
636- "ubuntu": {"fake-opt": {"eq": False}}
637- }
638+ linter.lint_rules["config"] = {"ubuntu": {"fake-opt": {"eq": False}}}
639 juju_status["applications"]["ubuntu"]["options"] = {"fake-opt": True}
640 juju_status["applications"]["ubuntu-host"] = juju_status["applications"].pop(
641 "ubuntu"
642@@ -720,21 +718,21 @@ applications:
643 }
644
645 def test_check_spaces_detect_mismatches(self, linter, mocker):
646- mock_log: mock.MagicMock = mocker.patch('jujulint.lint.Linter._log_with_header')
647+ mock_log: mock.MagicMock = mocker.patch("jujulint.lint.Linter._log_with_header")
648 linter.model.app_to_charm = self.check_spaces_example_app_charm_map
649
650 # Run the space check.
651 # Based on the above bundle, we should have exactly one mismatch.
652 linter.check_spaces(self.check_spaces_example_bundle)
653-
654+
655 # By default the mismatch should only trigger a warning, not an error.
656 errors = linter.output_collector["errors"]
657 assert len(errors) == 0
658 assert mock_log.call_count == 1
659- assert mock_log.mock_calls[0].kwargs['level'] == logging.WARN
660+ assert mock_log.mock_calls[0].kwargs["level"] == logging.WARN
661 assert mock_log.mock_calls[0].args[0] == (
662- 'Space binding mismatch: SpaceMismatch(telegraf-app:prometheus-client '
663- '(space external-space) != prometheus-app:target (space internal-space))'
664+ "Space binding mismatch: SpaceMismatch(prometheus-app:target "
665+ "(space internal-space) != telegraf-app:prometheus-client (space external-space))"
666 )
667
668 def test_check_spaces_enforce_endpoints(self, linter):
669@@ -749,7 +747,9 @@ applications:
670
671 # Enforce the opposite end of the relation.
672 # This should also generate an error.
673- linter.lint_rules["space checks"] = {"enforce endpoints": ["telegraf:prometheus-client"]}
674+ linter.lint_rules["space checks"] = {
675+ "enforce endpoints": ["telegraf:prometheus-client"]
676+ }
677 linter.check_spaces(self.check_spaces_example_bundle)
678 errors = linter.output_collector["errors"]
679 assert len(errors) == 2
680@@ -759,20 +759,24 @@ applications:
681
682 # Run the space check with prometheus:target endpoint enforced.
683 # This should generate an error.
684- linter.lint_rules["space checks"] = {"enforce relations": [["prometheus:target", "telegraf:prometheus-client"]]}
685+ linter.lint_rules["space checks"] = {
686+ "enforce relations": [["prometheus:target", "telegraf:prometheus-client"]]
687+ }
688 linter.check_spaces(self.check_spaces_example_bundle)
689 errors = linter.output_collector["errors"]
690 assert len(errors) == 1
691
692 # Reverse the relation's definition order.
693 # This should work the same way and also generate an error.
694- linter.lint_rules["space checks"] = {"enforce relations": [["telegraf:prometheus-client", "prometheus:target"]]}
695+ linter.lint_rules["space checks"] = {
696+ "enforce relations": [["telegraf:prometheus-client", "prometheus:target"]]
697+ }
698 linter.check_spaces(self.check_spaces_example_bundle)
699 errors = linter.output_collector["errors"]
700 assert len(errors) == 2
701
702 def test_check_spaces_ignore_endpoints(self, linter, mocker):
703- mock_log: mock.MagicMock = mocker.patch('jujulint.lint.Linter._log_with_header')
704+ mock_log: mock.MagicMock = mocker.patch("jujulint.lint.Linter._log_with_header")
705 linter.model.app_to_charm = self.check_spaces_example_app_charm_map
706
707 # Run the space check with prometheus:target endpoint ignored.
708@@ -785,19 +789,23 @@ applications:
709
710 # Enforce the opposite end of the relation.
711 # This should also generate an error.
712- linter.lint_rules["space checks"] = {"ignore endpoints": ["telegraf:prometheus-client"]}
713+ linter.lint_rules["space checks"] = {
714+ "ignore endpoints": ["telegraf:prometheus-client"]
715+ }
716 linter.check_spaces(self.check_spaces_example_bundle)
717 errors = linter.output_collector["errors"]
718 assert len(errors) == 0
719 assert mock_log.call_count == 0
720
721 def test_check_spaces_ignore_relations(self, linter, mocker):
722- mock_log: mock.MagicMock = mocker.patch('jujulint.lint.Linter._log_with_header')
723+ mock_log: mock.MagicMock = mocker.patch("jujulint.lint.Linter._log_with_header")
724 linter.model.app_to_charm = self.check_spaces_example_app_charm_map
725
726 # Run the space check with prometheus:target endpoint ignored.
727 # This should neither generate an error nor a warning.
728- linter.lint_rules["space checks"] = {"ignore relations": [["prometheus:target", "telegraf:prometheus-client"]]}
729+ linter.lint_rules["space checks"] = {
730+ "ignore relations": [["prometheus:target", "telegraf:prometheus-client"]]
731+ }
732 linter.check_spaces(self.check_spaces_example_bundle)
733 errors = linter.output_collector["errors"]
734 assert len(errors) == 0
735@@ -805,7 +813,9 @@ applications:
736
737 # Reverse the relation's definition order.
738 # This should work the same way and also not generate errors/warnings.
739- linter.lint_rules["space checks"] = {"ignore relations": [["telegraf:prometheus-client", "prometheus:target"]]}
740+ linter.lint_rules["space checks"] = {
741+ "ignore relations": [["telegraf:prometheus-client", "prometheus:target"]]
742+ }
743 linter.check_spaces(self.check_spaces_example_bundle)
744 errors = linter.output_collector["errors"]
745 assert len(errors) == 0
746diff --git a/tox.ini b/tox.ini
747index 3e9dbdf..843a442 100644
748--- a/tox.ini
749+++ b/tox.ini
750@@ -15,7 +15,7 @@ envlist = lintverbose, unit
751 skip_missing_interpreters = True
752
753 [testenv]
754-basepython = python3
755+basepython = python3.8
756 deps =
757 -r{toxinidir}/tests/requirements.txt
758 -r{toxinidir}/requirements.txt
759@@ -24,20 +24,34 @@ commands =
760 test: python3 -m unittest discover tests
761
762 [testenv:unit]
763-commands = pytest -v \
764- --cov=jujulint \
765- --cov-report=term \
766- --cov-report=annotate:tests/report/coverage-annotated \
767- --cov-report=html:tests/report/coverage-html \
768- --html=tests/report/index.html \
769- --junitxml=tests/report/junit.xml
770+commands =
771+ pytest -v \
772+ --cov=jujulint \
773+ --new-first \
774+ --last-failed \
775+ --last-failed-no-failures all \
776+ --cov-report=term-missing \
777+ --cov-report=annotate:tests/report/coverage-annotated \
778+ --cov-report=html:tests/report/coverage-html \
779+ --html=tests/report/index.html \
780+ --junitxml=tests/report/junit.xml
781 setenv = PYTHONPATH={toxinidir}/lib
782
783 [testenv:lint]
784 commands = flake8 jujulint --format=html --htmldir=tests/report/lint/ --tee
785
786 [testenv:lintverbose]
787-commands = flake8 jujulint
788+commands =
789+ flake8 {toxinidir}/jujulint {toxinidir}/tests
790+ black --check {toxinidir}/jujulint {toxinidir}/tests
791+ isort --check {toxinidir}/jujulint {toxinidir}/tests
792+
793+
794+[testenv:format-code]
795+commands =
796+ black {toxinidir}/jujulint {toxinidir}/tests
797+ isort {toxinidir}/jujulint {toxinidir}/tests
798+
799
800 [testenv:lintjunit]
801 commands = flake8 jujulint --format junit-xml --output-file=report/lint/junit.xml
802@@ -45,3 +59,6 @@ commands = flake8 jujulint --format junit-xml --output-file=report/lint/junit.xm
803 [pytest]
804 filterwarnings =
805 ignore::DeprecationWarning
806+
807+[isort]
808+profile = black

Subscribers

People subscribed via source and target branches

to all changes: