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

spring-cloud-dataflow-shell fails to parse kebab-case arguments #5172

Closed
michael-wirth opened this issue Jan 2, 2023 · 3 comments
Closed
Assignees
Labels
type/bug Is a bug report
Milestone

Comments

@michael-wirth
Copy link
Contributor

Description:

kebab-case arguments are not parsed correctly by the shell.

e.g the argument --metadata-uri does not resolve to it's value, but it works with camelCase syntax --metadataUri

Negative case with --metadata-uri (kebab-case)

Command..

app register --name paymentorderloader --type task 
--uri docker:code-registry.xxx.com/jobs/payment-order-loader:40.0.0-SNAPSHOT
--metadata-uri maven://ch.xxx.job:payment-order-loader:jar:metadata:40.0.0-SNAPSHOT
--force true`

.. resolves to
Arguments: [paymentorderloader, task, docker:code-registry.xxx.com/jobs/payment-order-loader:40.0.0-SNAPSHOT, --metadata-uri, true]

Log file:

org.springframework.shell.Shell          : Evaluate input with line=[app register --name paymentorderloader --type task --uri docker:code-registry.xxx.com/jobs/payment-order-loader:40.0.0-SNAPSHOT --metadata-uri maven://ch.xxx.eb.job:payment-order-loader:jar:metadata:40.0.0-SNAPSHOT --force true], command=[app register]
o.s.s.c.invocation.InvocableShellMethod  : Arguments: [paymentorderloader, task, docker:code-registry.xxx.com/jobs/payment-order-loader:40.0.0-SNAPSHOT, --metadata-uri, true]

Positive case with --metadataUri (camelCase)

Command..

app register --name paymentorderloader --type task 
--uri docker:code-registry.xxx.com/jobs/payment-order-loader:40.0.0-SNAPSHOT
--metadataUri maven://ch.xxx.job:payment-order-loader:jar:metadata:40.0.0-SNAPSHOT
--force true`

.. resolves to
Arguments: [paymentorderloader, task, docker:code-registry.xxx.com/jobs/payment-order-loader:40.0.0-SNAPSHOT, maven://ch.xxx.job:payment-order-loader:jar:metadata:40.0.0-SNAPSHOT, true]

Log file:

org.springframework.shell.Shell          : Evaluate input with line=[app register --name paymentorderloader --type task --uri docker:code-registry.xxx.com/jobs/payment-order-loader:40.0.0-SNAPSHOT --metadataUri maven://ch.xxx.job:payment-order-loader:jar:metadata:40.0.0-SNAPSHOT --force true], command=[app register]
o.s.s.c.invocation.InvocableShellMethod  : Arguments: [paymentorderloader, task, docker:code-registry.xxx.com/jobs/payment-order-loader:40.0.0-SNAPSHOT, maven://ch.xxx.job:payment-order-loader:jar:metadata:40.0.0-SNAPSHOT, true]

Release versions:
spring-cloud-dataflow-shell:2.10.0
tested with docker: bitnami/spring-cloud-dataflow-shell:2.10.0

@github-actions github-actions bot added the status/need-triage Team needs to triage and take a first look label Jan 2, 2023
@michael-wirth
Copy link
Contributor Author

Version 2.9.6 works with both cases

@markpollack markpollack added this to the 2.10.1 milestone Jan 5, 2023
@jvalkeal
Copy link
Collaborator

jvalkeal commented Jan 6, 2023

Compiling this bug to better format.

In 2.9.x:

dataflow:>app register --name timestamp1 --type task --uri maven://io.spring:timestamp-task:2.0.2 --metadata-uri maven://io.spring:timestamp-task:2.0.2
Successfully registered application 'task:timestamp1'

dataflow:>app list
╔═══╤══════╤═════════╤════╤══════════╗
║app│source│processor│sink│   task   ║
╠═══╪══════╪═════════╪════╪══════════╣
║   │      │         │    │timestamp1║
╚═══╧══════╧═════════╧════╧══════════╝

dataflow:>app info --type task --name timestamp1
Information about task application 'timestamp1':
Version: '2.0.2':
Default application version: 'true':
Resource URI: maven://io.spring:timestamp-task:2.0.2
╔══════════════════════════════╤══════════════════════════════╤══════════════════════════════╤══════════════════════════════╗
║         Option Name          │         Description          │           Default            │             Type             ║
╚══════════════════════════════╧══════════════════════════════╧══════════════════════════════╧══════════════════════════════╝


dataflow:>app register --name timestamp2 --type task --uri maven://io.spring:timestamp-task:2.0.2 --metadataUri maven://io.spring:timestamp-task:2.0.2
Option 'metadataUri' is not available for this command. Use tab assist or the "help" command to see the legal options

In 2.10.x:

dataflow:>app register --name timestamp1 --type task --uri maven://io.spring:timestamp-task:2.0.2 --metadata-uri maven://io.spring:timestamp-task:2.0.2

errors in server:

dataflow-server_1  | org.springframework.cloud.deployer.resource.support.ResourceNotResolvedException: a scheme (prefix) is required
dataflow-server_1  | 	at org.springframework.cloud.deployer.resource.support.DelegatingResourceLoader.getResource(DelegatingResourceLoader.java:92)
dataflow-server_1  | 	at org.springframework.cloud.dataflow.registry.support.AppResourceCommon.getMetadataResource(AppResourceCommon.java:261)
dataflow-server_1  | 	at org.springframework.cloud.dataflow.registry.service.DefaultAppRegistryService.getAppMetadataResource(DefaultAppRegistryService.java:309)
dataflow-server_1  | 	at org.springframework.cloud.dataflow.registry.service.DefaultAppRegistryService$$FastClassBySpringCGLIB$$a8bae4.invoke(<generated>)
dataflow-server_1  | 	at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:218)
dataflow-server_1  | 	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint(CglibAopProxy.java:793)
dataflow-server_1  | 	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
dataflow-server_1  | 	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:763)
dataflow-server_1  | 	at org.springframework.transaction.interceptor.TransactionInterceptor$1.proceedWithInvocation(TransactionInterceptor.java:123)
dataflow-server_1  | 	at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:388)
dataflow-server_1  | 	at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:119)
dataflow-server_1  | 	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
dataflow-server_1  | 	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:763)
dataflow-server_1  | 	at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:708)
dataflow-server_1  | 	at org.springframework.cloud.dataflow.registry.service.DefaultAppRegistryService$$EnhancerBySpringCGLIB$$885f842f.getAppMetadataResource(<generated>)
dataflow-server_1  | 	at org.springframework.cloud.dataflow.server.controller.AppRegistryController.lambda$null$1(AppRegistryController.java:439)
dataflow-server_1  | 	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.stream.ReferencePipeline$2$1.accept(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.stream.AbstractPipeline.copyInto(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.stream.ForEachOps$ForEachTask.compute(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.concurrent.CountedCompleter.exec(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.concurrent.ForkJoinTask.doInvoke(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.concurrent.ForkJoinTask.invoke(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateParallel(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateParallel(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.stream.AbstractPipeline.evaluate(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.stream.ReferencePipeline.forEach(Unknown Source)
dataflow-server_1  | 	at org.springframework.cloud.dataflow.server.controller.AppRegistryController.lambda$prefetchMetadata$2(AppRegistryController.java:436)
dataflow-server_1  | 	at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.concurrent.ForkJoinPool.scan(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
dataflow-server_1  | 	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)
dataflow-server_1  | Caused by: java.lang.IllegalArgumentException: a scheme (prefix) is required
dataflow-server_1  | 	at org.springframework.util.Assert.notNull(Assert.java:201)
dataflow-server_1  | 	at org.springframework.cloud.deployer.resource.support.DelegatingResourceLoader.getResource(DelegatingResourceLoader.java:79)
dataflow-server_1  | 	... 35 common frames omitted

then even app info errors in server. You can use --metadataUri:

dataflow:>app list
╔═══╤══════╤═════════╤════╤══════════╗
║app│source│processor│sink│   task   ║
╠═══╪══════╪═════════╪════╪══════════╣
║   │      │         │    │timestamp1║
╚═══╧══════╧═════════╧════╧══════════╝

dataflow:>app info --type task --name timestamp1
Application info is not available for task:timestamp1
dataflow:>app register --name timestamp2 --type task --uri maven://io.spring:timestamp-task:2.0.2 --metadataUri maven://io.spring:timestamp-task:2.0.2
Successfully registered application 'task:timestamp2'
dataflow:>app list
╔═══╤══════╤═════════╤════╤══════════╗
║app│source│processor│sink│   task   ║
╠═══╪══════╪═════════╪════╪══════════╣
║   │      │         │    │timestamp1║
║   │      │         │    │timestamp2║
╚═══╧══════╧═════════╧════╧══════════╝

dataflow:>app info --type task --name timestamp2
Information about task application 'timestamp2':
Version: '2.0.2':
Default application version: 'true':
Resource URI: maven://io.spring:timestamp-task:2.0.2
╔══════════════════════════════╤══════════════════════════════╤══════════════════════════════╤══════════════════════════════╗
║         Option Name          │         Description          │           Default            │             Type             ║
╚══════════════════════════════╧══════════════════════════════╧══════════════════════════════╧══════════════════════════════╝

@jvalkeal
Copy link
Collaborator

jvalkeal commented Jan 6, 2023

There's 2 or 3 bugs here.

  • Shell in 2.9.x was based on spring-shell 1.2.x and it hide completions for non-mandatory options and wrongly used --metadata-uri as kebab-case when we tried to use camelCase elsewhere. We missed this when in 2.10.x spring-shell were switched to 2.1.x.
  • Didn't look what happens if you force malformed metadata uri via api causing server error, but we need to do better on a shell side what comes for wrong options. aka not reporting wrong option --metadata-uri as it was accidentally switched to --metadataUri. Need to think if we want to support both to keep backward compatibility.

@jvalkeal jvalkeal added type/bug Is a bug report and removed status/need-triage Team needs to triage and take a first look labels Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Is a bug report
Projects
None yet
Development

No branches or pull requests

3 participants