Commit graph

5 commits

Author SHA1 Message Date
Vadim Stepanov
f977f9faee
Minor formatting changes (#2641)
# What this PR does

- Updates `black` and `flake8` to latest
- Removes `F541` from flake8 ignore (`F541 f-string is missing
placeholders`)
- Enables ["float to top"
option](https://pycqa.github.io/isort/docs/configuration/options.html#float-to-top)
for `isort`
2023-07-26 14:45:44 +01:00
Vadim Stepanov
b2f4ffb98a
apps.get_model -> import (#2619)
# What this PR does

Remove
[`apps.get_model`](https://docs.djangoproject.com/en/3.2/ref/applications/#django.apps.apps.get_model)
invocations and use inline `import` statements in places where models
are imported within functions/methods to avoid circular imports.

I believe `import` statements are more appropriate for most use cases as
they allow for better static code analysis & formatting, and solve the
issue of circular imports without being unnecessarily dynamic as
`apps.get_model`. With `import` statements, it's possible to:

- Jump to model definitions in most IDEs
- Automatically sort inline imports with `isort`
- Find import errors faster/easier (most IDEs highlight broken imports)
- Have more consistency across regular & inline imports when importing
models

This PR also adds a flake8 rule to ban imports of `django.apps.apps`, so
it's harder to use `apps.get_model` by mistake (it's possible to ignore
this rule by using `# noqa: I251`). The rule is not enforced on
directories with migration files, because `apps.get_model` is often used
to get a historical state of a model, which is useful when writing
migrations ([see this SO answer for more
details](https://stackoverflow.com/a/37769213)). So `apps.get_model` is
considered OK in migrations (even necessary in some cases).

## Checklist

- [x] Unit, integration, and e2e (if applicable) tests updated
- [x] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)
2023-07-25 09:43:23 +00:00
Joey Orlando
014a9c2ec2
allow the POST incoming alert endpoints to queue create_alert tasks independent of the database status (#1896)
# What this PR does

https://www.loom.com/share/18cc445117de4895a10892d56c7d3699

In preparation to upgrade our cloud databases, this PR makes some minor
changes which, after testing locally, allowed the `POST
/<integration_type>/<alert_channel_key>` endpoints to successfully
receive incoming alerts and queue the celery tasks.

I've tested all of the defined `POST
/integrations/v1/<integration_type>/<alert_channel_key>` endpoints by
sending `POST` requests to an integrations' URL while the MySQL database
was down, bringing the database back up, and ensuring the alerts were
created.

## Some other findings
- the integration heartbeat endpoints will not work as we interact w/
the database to persist the incoming heartbeat instance
- if the integration was created in the last 180 seconds, incoming
alerts will fail due to the way we cache the integration IDs
([code](https://github.com/grafana/oncall/blob/dev/engine/apps/integrations/mixins/alert_channel_defining_mixin.py#L47-L50))
- The `create_alert` celery task is set to `max_retries=None` and
`retry_backoff=True`. This means that the queued tasks will continue
retrying forever w/ an exponential backoff, until the alerts can be
created in the database (ie. when the database is back online).

## Checklist

- [ ] Unit, integration, and e2e (if applicable) tests updated (N/A)
- [ ] Documentation added (or `pr:no public docs` PR label added if not
required) (N/A)
- [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required) (N/A)
2023-05-10 12:36:23 +00:00
Innokentii Konstantinov
7bad073626
Remove OSS_INSTALATION env var (#881)
It's a duplicate of LICENSE env var

**What this PR does**:
Remove OSS_INSTALLATION env var in favour of LICENSE env var. Also, I
refactored features tests a little. From my point of view it makes
little sense to test if all features are disabled or enabled. Better to
test specific use-case (e.g. oss installation).
Also to test that all features are disabled it is needed to set LICENSE
equals cloud license, which makes test confusing.

**Checklist**
- [x] Tests updated
- [ ] Documentation added
- [ ] `CHANGELOG.md` updated
2023-03-07 11:07:42 +00:00
Yulya Artyukhina
381520ee13
Get rid of installation token + add a bunch of tests (#624)
* Get rid of installation token (for OSS installations)

This is done by being required to supply the grafana API URL as an
environment variable on the backend. Additionally, optionally an OnCall
API URL environment variable can be passed in to the frontend (this basically
allows completely skipping the need to configure anything).
- deduplicated a lot of the sync logic on the frontend + made
error message more useful and consistent
- Split PluginConfigPage component into several subcomponents
(making it easier to test each individual component)
- Moved RootWithLoader (from plugin/GrafanaPluginRootPage) into its own
subcomponent (making it easier to test)
- Added tests for pre-existing components that were touched:
  - PluginConfigPage component (and its new subcomponents)
  - state/plugin and state/rootBaseStore functions
  - apps.grafana_plugin django app

Helm changes:
- add GRAFANA_API_URL to oncall.env
- some yaml autoformatting changes
- remove reference to python manage.py issue_invite_for_the_frontend --override

Co-authored-by: Joey Orlando <joseph.t.orlando@gmail.com>
2022-11-21 16:26:00 +01:00