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

Compatibility with upcoming deprecation of plugin setting kind: hidden and kind: password #1418

Open
ReubenFrankel opened this issue Jul 7, 2023 · 6 comments
Assignees

Comments

@ReubenFrankel
Copy link
Contributor

ReubenFrankel commented Jul 7, 2023

When meltano/meltano#7733 and meltano/meltano#7874 are released in a future Meltano version, kind: hidden and kind: password will become deprecated and show a warning message if existing setting definitions use them.

image

Problem

The deprecation message will show until the plugin definition is updated (i.e. plugin .lock file no longer contains deprecated kinds) on the Hub and meltano lock --update is run. Since all versions of Meltano reference the same Hub instance, removing the deprecated kinds for all plugin definitions would potentially break all but the latest versions of Meltano if a user was to ever update their plugin .lock files (they don't know anything about the new hidden: true and sensitive: true setting properties, unless the features are back-ported).

Changes (solution permitting)

kind: hidden

- name: oauth_token_proxy_url
  kind: hidden
  value: https://proxy.host/api/oauth/token

changes to

- name: oauth_token_proxy_url
  hidden: true
  value: https://proxy.host/api/oauth/token

kind: password

- name: api_key
  kind: password

changes to

- name: api_key
  sensitive: true

kind as actual data type

Consider providing actual expected data types as kind now that this is possible, if applicable for a plugin. I imagine that the majority of plugins will not require this, but consider the following setting:

- name: poll_interval_ms
  kind: hidden
  value: 5000

should probably change to

- name: poll_interval_ms
  kind: integer
  hidden: true
  value: 5000
@pnadolny13
Copy link
Contributor

@ReubenFrankel thanks for the write up!

How would we do this without breaking existing user's projects? We dont have yaml schema versioning of plugins on the hub so I'm not sure how we'd be able to transition to this new schema. It seems like we should be able to add the new keys to the definitions but we couldnt change the kind otherwise any non-latest meltano versions would stop treating that setting as sensitive. When would we be safe to remove them so the deprecation warnings go away?

@ReubenFrankel
Copy link
Contributor Author

One option might be to not show the deprecation messages or put them behind a feature flag for now (like how .lock files were introduced before Meltano v2). Perhaps they could be officially deprecated as of v3 and eventually removed in v4, which provides a good amount of time to come up with a versionable spec for the Hub.

The other way would be to backport the changes to older Meltano versions as patch releases. I don't think this is something Meltano has done before - probably for good reason since it feels like a violation of the principles of semantic versioning IMO.

@tayloramurphy
Copy link
Collaborator

@meltano/engineering would appreciate some thoughts on this. Seems like the conversation in #1205 (comment) is very relevant.

@pnadolny13
Copy link
Contributor

This is what I think would happen based on what steps we take on the hub:

Update the hub metadata to reflect this new change:

  • meltano versions >=2.0.0<2.20.0 (future release with these changes) stop treating password fields as secrets. They'll get put in meltano.yml instead of .env because sensitive=true is the flag for passwords now but the old code is still looking for kind=password which isnt the case anymore because kind=string in the metadata.
  • meltano versions >=2.0.0<2.20.0 potentially throws exceptions because these new attributes are present. I doubt this is the case but we should confirm. @ReubenFrankel I imagine you tested this, do you know?
  • meltano >=2.20.0 everything is great! The new feature is present.

Don't update the hub metadata to reflect this new change:

  • meltano versions >=2.0.0<2.20.0 - Nothing changed so everything stays the same
  • meltano >=2.20.0 - Nothing changed but the new feature was merged so a deprecation warning is logged. Also the new settings arent available on the hub so users would have to manually update their lock files to make them go away. Newly added plugins will cause warnings. Users can ignore or silence the warning but then the new feature isnt used.

would appreciate some thoughts on this. Seems like the conversation in #1205 (comment) is very relevant.

@tayloramurphy I think the challenge here is slightly different than #1205 (comment) because that is trying to make sure the plugin python package and plugin metadata content pulled from the hub are in sync. This is versioning between the hub plugin metadata schema and the meltano package version that consumes it. Versioning in this case would mean updating our hub API route /meltano/api/v1/plugins/index to v2 like /meltano/api/v2/plugins/index where the response metadata schema is now slightly different but breaking.

Perhaps they could be officially deprecated as of v3 and eventually removed in v4, which provides a good amount of time to come up with a versionable spec for the Hub.

I think this is probably the path forward but it requires us to have multiple hub API versions. We could keep v1 up basically indefinitely because all meltano versions >=2.0.0<2.20.0 use it and we could also serve a v2 version with these new breaking changes that meltano versions >=2.20.0 would use. Eventually we'd want to deprecate v1 but we'd probably want to set a date far in the future and give lots of warnings to not disrupt users who update versions slowly. I wonder how much work it is to update the hub to serve multiple API versions.

@ReubenFrankel
Copy link
Contributor Author

@pnadolny13 I tested with 2.19.1 with a modified .lock file - no problems:

reuben@reuben-Inspiron-14-5425:/tmp$ meltano init p
Creating .meltano folder
created .meltano in /tmp/p/.meltano
Creating project files...
  p/
   |-- meltano.yml
   |-- README.md
   |-- requirements.txt
   |-- output/.gitignore
   |-- .gitignore
   |-- extract/.gitkeep
   |-- load/.gitkeep
   |-- transform/.gitkeep
   |-- analyze/.gitkeep
   |-- notebook/.gitkeep
   |-- orchestrate/.gitkeep
Creating system database...  Done!



                          ████   █████
                         ░░███  ░░███
 █████████████    ██████  ░███  ███████    ██████   ████████    ██████
░░███░░███░░███  ███░░███ ░███ ░░░███░    ░░░░░███ ░░███░░███  ███░░███
 ░███ ░███ ░███ ░███████  ░███   ░███      ███████  ░███ ░███ ░███ ░███
 ░███ ░███ ░███ ░███░░░   ░███   ░███ ███ ███░░███  ░███ ░███ ░███ ░███
 █████░███ █████░░██████  █████  ░░█████ ░░████████ ████ █████░░██████
░░░░░ ░░░ ░░░░░  ░░░░░░  ░░░░░    ░░░░░   ░░░░░░░░ ░░░░ ░░░░░  ░░░░░░



Your project has been created!

Meltano Environments initialized with dev, staging, and prod.
To learn more about Environments visit: https://docs.meltano.com/concepts/environments

Next steps:
  cd p
  Visit https://docs.meltano.com/getting-started/part1 to learn where to go from here
reuben@reuben-Inspiron-14-5425:/tmp$ cd p
reuben@reuben-Inspiron-14-5425:/tmp/p$ meltano add extractor tap-auth0
Added extractor 'tap-auth0' to your Meltano project
Variant:	matatika (default)
Repository:	https://github.com/Matatika/tap-auth0
Documentation:	https://hub.meltano.com/extractors/tap-auth0--matatika

Installing extractor 'tap-auth0'...
Installed extractor 'tap-auth0'

To learn more about extractor 'tap-auth0', visit https://hub.meltano.com/extractors/tap-auth0--matatika
reuben@reuben-Inspiron-14-5425:/tmp/p$ cp ~/Documents/taps/tap-auth0/.env .
reuben@reuben-Inspiron-14-5425:/tmp/p$ nano plugins/extractors/tap-auth0--matatika.lock 
reuben@reuben-Inspiron-14-5425:/tmp/p$ bat plugins/extractors/tap-auth0--matatika.lock 
reuben@reuben-Inspiron-14-5425:/tmp/p$ cat plugins/extractors/tap-auth0--matatika.lock 
{
  "plugin_type": "extractors",
  "name": "tap-auth0",
  "namespace": "tap_auth0",
  "variant": "matatika",
  "label": "Auth0",
  "docs": "https://hub.meltano.com/extractors/tap-auth0--matatika",
  "repo": "https://github.com/Matatika/tap-auth0",
  "pip_url": "git+https://github.com/Matatika/tap-auth0.git",
  "description": "Authentication and Authorization Platform",
  "logo_url": "https://hub.meltano.com/assets/logos/extractors/auth0.png",
  "capabilities": [
    "catalog",
    "discover",
    "state",
    "about",
    "stream-maps"
  ],
  "settings_group_validation": [
    [
      "client_id",
      "client_secret",
      "domain"
    ]
  ],
  "settings": [
    {
      "name": "client_id",
      "sensitive": true,
      "label": "Client ID",
      "description": "App client ID"
    },
    {
      "name": "client_secret",
      "sensitive": true,
      "label": "Client secret",
      "description": "App client secret"
    },
    {
      "name": "domain",
      "label": "Domain",
      "description": "Tenant domain"
    },
    {
      "name": "job_poll_interval_ms",
      "kind": "integer",
      "hidden": true,
      "value": 2000,
      "label": "Job poll interval",
      "description": "Job poll interval (ms)"
    },
    {
      "name": "job_poll_max_count",
      "kind": "integer",
      "hidden": true,
      "value": 10,
      "label": "Maximum job poll count",
      "description": "Maximum job poll count"
    }
  ]
}
reuben@reuben-Inspiron-14-5425:/tmp/p$ meltano config tap-auth0 list
2023-07-19T14:08:28.517695Z [info     ] The default environment 'dev' will be ignored for `meltano config`. To configure a specific environment, please use the option `--environment=<environment name>`.
client_id [env: TAP_AUTH0_CLIENT_ID] current value: '<REDACTED>' (from the TAP_AUTH0_CLIENT_ID variable in `.env`)
	Client ID: App client ID
client_secret [env: TAP_AUTH0_CLIENT_SECRET] current value: '<REDACTED>' (from the TAP_AUTH0_CLIENT_SECRET variable in `.env`)
	Client secret: App client secret
domain [env: TAP_AUTH0_DOMAIN] current value: 'matatika-staging.eu.auth0.com' (from the TAP_AUTH0_DOMAIN variable in `.env`)
	Domain: Tenant domain
job_poll_interval_ms [env: TAP_AUTH0_JOB_POLL_INTERVAL_MS] (default: 2000) current value: 5000 (from the TAP_AUTH0_JOB_POLL_INTERVAL_MS variable in `.env`)
	Job poll interval: Job poll interval (ms)
job_poll_max_count [env: TAP_AUTH0_JOB_POLL_MAX_COUNT] current value: 10 (default)
	Maximum job poll count: Maximum job poll count

To learn more about extractor 'tap-auth0' and its settings, visit https://hub.meltano.com/extractors/tap-auth0--matatika
reuben@reuben-Inspiron-14-5425:/tmp/p$ meltano config tap-auth0 test
2023-07-19T14:08:35.495434Z [info     ] The default environment 'dev' will be ignored for `meltano config`. To configure a specific environment, please use the option `--environment=<environment name>`.
Plugin configuration is valid

(somewhat ironically leaked sensitive credentials from config list posting this the first time)

@tayloramurphy
Copy link
Collaborator

We decided after Office Hours that @edgarrmondragon was going to take a spike to see what's required to update the Hub API version. @edgarrmondragon can you create an issue for that so we can track?

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

No branches or pull requests

3 participants