Skip to content

Comments

feat(config): stateless mode#147

Closed
hekike wants to merge 5 commits intonextfrom
feat/stateless
Closed

feat(config): stateless mode#147
hekike wants to merge 5 commits intonextfrom
feat/stateless

Conversation

@hekike
Copy link
Contributor

@hekike hekike commented Aug 7, 2023

This PR adds:

  • documents stateless mode
  • in stateless mode ignore static meter configs (log a warning if any defined)
  • enable/disable meter create and delete
  • enable/disable namespace create
  • enable schema registry via enable flag instead of checking on URL content
  • update server start log with components' state

@hekike hekike requested a review from tothandras August 7, 2023 18:49
@hekike hekike changed the base branch from main to next August 7, 2023 18:49
@hekike hekike added kind/feature New feature or request area/api release-note/feature Release note: Exciting New Features labels Aug 7, 2023
@hekike hekike requested a review from sagikazarmark August 8, 2023 06:39
Comment on lines +78 to +81
c.Server.DisableNamespaceCreate = true
c.Server.DisableNamespaceDelete = true
c.Server.DisableMeterCreate = true
c.Server.DisableMeterDelete = true
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not gonna work:

  • First of all, Validate shouldn't change the state
  • Secondly, the receiver is not a pointer which means any change within this function would be lost.

A better pattern to do config processing is creating a Process function and calling that prior to validate:

func (c *configuration) Process() {
    // ...
}

Call this function in main before validate.

Comment on lines +161 to +164
DisableNamespaceCreate bool
DisableNamespaceDelete bool
DisableMeterCreate bool
DisableMeterDelete bool
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's leave these here for now, but I think logically, these belong under namespace and meter specific configuration.

The config controls the feature, not the HTTP layer. I'm okay with merging this PR as it is, but I will probably send another PR refactoring this a bit.

### Config Mode

Config mode is the default and recommended mode in OpenMeter. In this mode you can define meters via the `config.yaml`.
As the state is defined in the config, namespace and meter management HTTP endpoints are disabled.
Copy link
Contributor

Choose a reason for hiding this comment

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

Since namespaces are not stored anyway, does this actually make sense?


### Stateless Mode

You can enable stateless mode via `stateless: true` in the config. In this mode meters defined in the `config.yaml` are ignored. In stateless mode you can define namespaces and meters via the HTTP API but these states are not persisted and after restarting OpenMeter they will be lost. Resources created by OpenMeter like Kafka topics, KSQL tables etc. will may remain persisted.
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, namespaces are not stored anyway (but do not get lost either). Does it make sense to call that out this way?

@sagikazarmark sagikazarmark deleted the branch next August 20, 2023 21:37
@hekike hekike deleted the feat/stateless branch October 10, 2023 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/api kind/feature New feature or request release-note/feature Release note: Exciting New Features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants