Apply default version to a serverless config #166
Conversation
iwysiu
requested review from
sarahzinger and
idastambuk
and removed request for
a team
|
Backend code coverage report for PR #166 |
|
Frontend code coverage report for PR #166
|
|
Levitate is-compatible report: 🔍 Resolving @grafana/data@latest... 🔬 Checking compatibility between ./src/module.ts and @grafana/data@9.5.2... 🔬 Checking compatibility between ./src/module.ts and @grafana/ui@9.5.2... 🔬 Checking compatibility between ./src/module.ts and @grafana/runtime@9.5.2... 🔬 Checking compatibility between ./src/module.ts and @grafana/e2e-selectors@9.5.2... ✔️ ./src/module.ts appears to be compatible with @grafana/data,@grafana/ui,@grafana/runtime,@grafana/e2e-selectors |
There was a problem hiding this comment.
I'm thinking in the future we will remove the feature toggle but still need to set a default option for serverless but in that case we won't need to do all of this right since the version/flavor are more obvious? Maybe we should write the code for serverless as a separate if conditional that we can keep as is after we remove the feature flag, wdyt?
I think this is what you meant? In the long term, we should probably refactor the code to handle serverless cases without a dummy version, but I figured that could wait until we decide whatever refactoring we want to do to opensearch. |
| if (options.jsonData.serverless) { | ||
| flavor = flavor || Flavor.OpenSearch; | ||
| version = version || '1.0.0'; | ||
| } |
There was a problem hiding this comment.
so why is this an Or case? I thought in serverless was always the same?
There was a problem hiding this comment.
done, it was in the old code and I didn't think about it
An edge case that I didn't catch while developing. It can be reproduced by using the aws provisioner app to provision a serverless OpenSearch version (we have one in
US East (Ohio)) and opening and trying to save the config. Before this fix, we would get theNo version seterror, but this applys a version when the config editor is open (which is what it did before the detecting version change). We don't really have any tests for this code path at the moment, but I can whip together acoerceOptionstest if we think we need one.