fix bug where -n and -k was mutually exclusive #281

bcwu · 2022-09-15T16:09:16Z

Description

1.10.0 added an erroneous exclusion that prevented users from using -n while also entering -k (or have CONNECT_API_KEY set in their environment variable). This PR fixes the exclusion. Now only -n and -s, and shinyapps options are mutually exclusive.

fixes #280
fixes connect #22150
fixes vetiver #110

mmarchetti

Do the insecure and cacert options work correctly if you specify them along with a saved server? I'd expect them to override the saved server's settings.

bcwu · 2022-09-15T21:57:17Z

Do the insecure and cacert options work correctly if you specify them along with a saved server? I'd expect them to override the saved server's settings.

~~fixed by b18bf74~~

Summary: stored > cli > environment

Per discussion, to facilitate existing user workflows, we will make non-empty stored credentials take precedence over CLI and environment credential variables. This will revert the behavior back to version <= 1.9.x.

We will add a warning when users enter a credential field while a non-empty stored credential field already exist, telling them that their CLI command or environment variable will be ignored.
For example, a user may have CONNECT_API_KEY set in their environment variable to run connect but would like to deploy to a completely different Connect instance that they have saved with a corresponding API key. In this case, rsconnect-python will show a warning. The warning is triggered in this case because the CONNECT_API_KEY in the user's environment is overlapping with a stored key, and that the CONNECT_API_KEY will be ignored while the stored credentials are used.
If the user leaves certain credential fields empty when they were adding a server to the store, then they will be able to provide those specific empty credential fields through the CLI or environment variables.
- We still validate the api key at initial add step, but a user may provide a ca cert (-c) at run time

This reverts commit b18bf74.

This reverts commit 5860b16.

This reverts commit cd2a678.

This reverts commit 20b00cf.

kgartland-rstudio · 2022-09-19T19:50:38Z

on the rsconnect-python bcwu-nameserverfix branch, I'm still hitting the same error vetiver was hitting here rstudio/vetiver-python#110.

rsconnect deploy bokeh -n fuzz -s https://rsc.radixu.com/ ./top-5-income-share-bokeh/
Error: -n/--name cannot be specified in conjunction with options -s/--server

kgartland-rstudio · 2022-09-19T19:51:43Z

I also found a new issue related to storing creds:

#283

kgartland-rstudio · 2022-09-19T19:57:14Z

When deploying with -k and -n we print a line of code: self.setup_remote_server( before the deployment logs:

rsconnect deploy bokeh -n dogfood -k notavalidapikey ./top-5-income-share-bokeh/
/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/rsconnect/api.py:341: UserWarning: Connect will use non-empty stored credentials. CLI & environment credentials are ignored.
self.setup_remote_server(
Validating server... [OK]
Validating app mode... [OK]
Making bundle ... [OK]
Deploying bundle ... [OK]
Saving deployed information... [OK]

bcwu · 2022-09-19T20:37:48Z

I also found a new issue related to storing creds:

#283

fixed by 7a14e71

When deploying with -k and -n we print a line of code: self.setup_remote_server( before the deployment logs:

rsconnect deploy bokeh -n dogfood -k notavalidapikey ./top-5-income-share-bokeh/
/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/rsconnect/api.py:341: UserWarning: Connect will use non-empty stored credentials. CLI & environment credentials are ignored.
self.setup_remote_server(
Validating server... [OK]
Validating app mode... [OK]
Making bundle ... [OK]
Deploying bundle ... [OK]
Saving deployed information... [OK]

The UserWarning is intentional. Although it can probably show up better.

kgartland-rstudio · 2022-09-19T20:40:05Z

The UserWarning is intentional. Although it can probably show up better.

I was referring to the: self.setup_remote_server( line before the deployment logs start.

bcwu · 2022-09-19T20:44:23Z

The UserWarning is intentional. Although it can probably show up better.

I was referring to the: self.setup_remote_server( line before the deployment logs start.

Oh ok, gotcha. This should be better: c163e89

kgartland-rstudio · 2022-09-19T20:50:09Z

Do we need all of this for a warning?

rsconnect deploy bokeh -n dogfood -k notavalidapikey ./top-5-income-share-bokeh/
/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/rsconnect/api.py:421: UserWarning: Connect will use non-empty stored credentials. CLI & environment credentials are ignored.
warn("Connect will use non-empty stored credentials. CLI & environment credentials are ignored.")

Can we just print out only the necessary text, i.e:

Connect will use non-empty stored credentials. CLI & environment credentials are ignored

The warning doesn't stand out very well against all the text.

kgartland-rstudio · 2022-09-19T20:57:05Z

Adding a server with no api key now fails. Is that the intended fix for #283?

-> rsconnect add -n dogfood -s https://rsc.radixu.com/
Checking https://rsc.radixu.com/... [OK]
Checking API key... [ERROR]
Error: The specified API key is not valid.

I also tried to add a shiny account and I got this error so I think that fix may have broken shiny.io deployments:

-> rsconnect add -n dogfood -s https://rsc.radixu.com/ --account blue --token notarealtoken --secret notarealsecret
Traceback (most recent call last):
File "/Users/kgartland/.pyenv/versions/rsconnect-tag/bin/rsconnect", line 8, in
sys.exit(cli())
File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/click/core.py", line 1130, in call
return self.main(*args, **kwargs)
File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/click/core.py", line 1055, in main
rv = self.invoke(ctx)
File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/click/core.py", line 1657, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/click/core.py", line 1404, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/click/core.py", line 760, in invoke
return __callback(*args, **kwargs)
File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/rsconnect/main.py", line 157, in wrapper
return func(*args, **kwargs)
File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/rsconnect/main.py", line 323, in add
validation.validate_connection_options(
File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/rsconnect/validation.py", line 35, in validate_connection_options
raise RSConnectException(
rsconnect.exception.RSConnectException: Connect options (-k/--api-key) may not be passed alongside shinyapps.io options (-T/--token, -S/--secret, -A/--account).

bcwu · 2022-09-19T21:08:40Z

Adding a server with no api key now fails. Is that the intended fix for #283?

-> rsconnect add -n dogfood -s https://rsc.radixu.com/
Checking https://rsc.radixu.com/... [OK]
Checking API key... [ERROR]
Error: The specified API key is not valid.

I also tried to add a shiny account and I got this error so I think that fix may have broken shiny.io deployments:

-> rsconnect add -n dogfood -s https://rsc.radixu.com/ --account blue --token notarealtoken --secret notarealsecret
Traceback (most recent call last):
File "/Users/kgartland/.pyenv/versions/rsconnect-tag/bin/rsconnect", line 8, in
sys.exit(cli())
File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/click/core.py", line 1130, in call
return self.main(*args, **kwargs)
File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/click/core.py", line 1055, in main
rv = self.invoke(ctx)
File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/click/core.py", line 1657, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/click/core.py", line 1404, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/click/core.py", line 760, in invoke
return __callback(*args, **kwargs)
File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/rsconnect/main.py", line 157, in wrapper
return func(*args, **kwargs)
File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/rsconnect/main.py", line 323, in add
validation.validate_connection_options(
File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/rsconnect/validation.py", line 35, in validate_connection_options
raise RSConnectException(
rsconnect.exception.RSConnectException: Connect options (-k/--api-key) may not be passed alongside shinyapps.io options (-T/--token, -S/--secret, -A/--account).

It looks like it is getting the API key from somewhere. Do you have CONNECT_API_KEY saved as an environment variable?

kgartland-rstudio · 2022-09-19T21:12:41Z

It looks like it is getting the API key from somewhere. Do you have CONNECT_API_KEY saved as an environment variable?

Yes, that's what it was. I forgot I was testing that earlier.

bcwu · 2022-09-19T21:17:23Z

It looks like it is getting the API key from somewhere. Do you have CONNECT_API_KEY saved as an environment variable?

Yes, that's what it was. I forgot I was testing that earlier.

This might also be a bug, because a user may be intentionally adding a server with no API key but unwittingly use CONNECT_API_KEY set in their environment.

kgartland-rstudio · 2022-09-20T13:18:33Z

This might also be a bug, because a user may be intentionally adding a server with no API key but unwittingly use CONNECT_API_KEY set in their environment.

FWIW, I haven't seen this documented anywhere and didn't know we used that variable for anything.

kgartland-rstudio · 2022-09-21T15:41:58Z

List of issues:

This reverts commit 6cb4d6f.

bcwu · 2022-09-27T20:16:06Z

on the rsconnect-python bcwu-nameserverfix branch, I'm still hitting the same error vetiver was hitting here rstudio/vetiver-python#110.

rsconnect deploy bokeh -n fuzz -s https://rsc.radixu.com/ ./top-5-income-share-bokeh/
Error: -n/--name cannot be specified in conjunction with options -s/--server

The error message "RSConnectException: -n/--name cannot be specified in conjunction with options -s/--server" in vetiver #110 was just the symptom. 5ddd7ad fixes the underlying issue.

-n and -s were mutually exclusive even in past versions, see here.

kgartland-rstudio · 2022-09-29T13:33:42Z

When adding a Connect server I get this now:

-> rsconnect add -n testdogfood -s https://rsc.radixu.com/ -k ${DOGFOOD_API_KEY}
Detected the following inputs:
    name: COMMANDLINE
    server: COMMANDLINE
    api_key: COMMANDLINE
    insecure: DEFAULT
Checking https://rsc.radixu.com/...              [OK]
Checking API key...                              [OK]
Added Connect server "testdogfood" with URL https://rsc.radixu.com/

Then when I create a CONNECT-API-KEY variable, I get:

-> CONNECT_API_KEY=${DOGFOOD_API_KEY} rsconnect add -n testdogfood -s https://rsc.radixu.com/
Detected the following inputs:
    name: COMMANDLINE
    server: COMMANDLINE
    api_key: ENVIRONMENT
    insecure: DEFAULT
Checking https://rsc.radixu.com/...              [OK]
Checking API key...                              [OK]
Added Connect server "testdogfood" with URL https://rsc.radixu.com/

I think for most use-cases this is a little too much and would cause confusion. Can we put that info in the verbose logging instead? My first instinct if something isn't working as expected would be to add the verbose flag and then check the output there. Then we can elaborate a bit more too, something like:

rsconnect-python detected: 
--name entered from Command Line
--server entered from Command Line
--api_key from Environment Variable CONNECT_API_KEY

bcwu · 2022-09-29T13:54:19Z

'Add' is one of those commands that is use and forget but has huge number of downstream consequences. So showing the input detection at the initial step will prevent a lot of the environment variable issues our users encountered.

kgartland-rstudio · 2022-09-29T14:18:24Z

We now allow adding a server without a --name on this branch:

-> rsconnect add -s https://rsc.radixu.com/ -k ${DOGFOOD_API_KEY}
Detected the following inputs:
    server: COMMANDLINE
    api_key: COMMANDLINE
    insecure: DEFAULT
Checking https://rsc.radixu.com/...              [OK]
Checking API key...                              [OK]
Added Connect server "None" with URL https://rsc.radixu.com/

When you list the rsconnect servers it errors.

-> rsconnect list
Traceback (most recent call last):
  File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/rsconnect/actions.py", line 80, in cli_feedback
    yield
  File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/rsconnect/main.py", line 358, in list_servers
    servers = server_store.get_all_servers()
  File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/rsconnect/metadata.py", line 278, in get_all_servers
    return self._get_sorted_values(lambda s: s["name"])
  File "/Users/kgartland/.pyenv/versions/3.8.2/envs/rsconnect-tag/lib/python3.8/site-packages/rsconnect/metadata.py", line 144, in _get_sorted_values
    return sorted(self._data.values(), key=sort_by)
TypeError: '<' not supported between instances of 'NoneType' and 'str'
Internal error: '<' not supported between instances of 'NoneType' and 'str'

The server.json file looks like this:

    "null": {
        "name": null,
        "url": "https://rsc.radixu.com/",
        "api_key": "REDACTED",
        "insecure": false,
        "ca_cert": null
    }
}

The same command on 1.10.0 errors properly:

-> rsconnect add -s https://rsc.radixu.com/ -k ${DOGFOOD_API_KEY}
Usage: rsconnect add [OPTIONS]
Try 'rsconnect add --help' for help.

Error: Missing option '--name' / '-n'.

bcwu · 2022-09-29T14:27:18Z

We now allow adding a server without a --name on this branch:

fixed by f8ea20e

fix bug where -n and -k was mutually exclusive

5ddd7ad

bcwu requested review from mmarchetti and kgartland-rstudio September 15, 2022 16:11

mmarchetti approved these changes Sep 15, 2022

View reviewed changes

allow partial override of stored from cli

b18bf74

bcwu added 10 commits September 16, 2022 11:07

Revert "allow partial override of stored from cli"

1439bd9

This reverts commit b18bf74.

test modified prelease

20b00cf

cat mock_connect.log

cd2a678

correct mock_connect.log location

5860b16

Revert "correct mock_connect.log location"

1859177

This reverts commit 5860b16.

Revert "cat mock_connect.log"

3e56bb4

This reverts commit cd2a678.

Revert "test modified prelease"

2ad0f5b

This reverts commit 20b00cf.

add warning when stored credentials already exist

0444591

Merge branch 'master' into bcwu-nameserverfix

e9393aa

shorten warning length for flake8

05d256a

isabelizimm mentioned this pull request Sep 19, 2022

name cannot be specified with server on RSConnect rstudio/vetiver-python#110

Closed

bcwu added 2 commits September 19, 2022 16:32

make list server check not error out on None fields

8695b00

correct display of "API key is saved" message

7a14e71

remove stack level from warning

c163e89

update message and output to logging

ee46ae8

handle no logger case

46ba9d7

make logger required to show warning

366dc07

bcwu added 3 commits September 27, 2022 14:42

Show detected inputs when adding

6cb4d6f

Update credential overlap warning messages

4dfb46c

Revert "Show detected inputs when adding"

aeb8db4

This reverts commit 6cb4d6f.

Show detected inputs when adding

e5b465c

add back -n as required on add

f8ea20e

bcwu merged commit 6e3a84d into master Sep 29, 2022

bcwu deleted the bcwu-nameserverfix branch September 29, 2022 16:08

isabelizimm mentioned this pull request Oct 19, 2023

deploy_rsconnect causes kernel to die unexpectedly rstudio/vetiver-python#198

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix bug where -n and -k was mutually exclusive #281

fix bug where -n and -k was mutually exclusive #281

bcwu commented Sep 15, 2022 •

edited

Loading

mmarchetti left a comment

bcwu commented Sep 15, 2022 •

edited

Loading

kgartland-rstudio commented Sep 19, 2022

kgartland-rstudio commented Sep 19, 2022 •

edited

Loading

kgartland-rstudio commented Sep 19, 2022 •

edited

Loading

bcwu commented Sep 19, 2022

kgartland-rstudio commented Sep 19, 2022

bcwu commented Sep 19, 2022

kgartland-rstudio commented Sep 19, 2022

kgartland-rstudio commented Sep 19, 2022

bcwu commented Sep 19, 2022

kgartland-rstudio commented Sep 19, 2022

bcwu commented Sep 19, 2022

kgartland-rstudio commented Sep 20, 2022

kgartland-rstudio commented Sep 21, 2022

bcwu commented Sep 27, 2022 •

edited

Loading

kgartland-rstudio commented Sep 29, 2022

bcwu commented Sep 29, 2022

kgartland-rstudio commented Sep 29, 2022

bcwu commented Sep 29, 2022

fix bug where -n and -k was mutually exclusive #281

fix bug where -n and -k was mutually exclusive #281

Conversation

bcwu commented Sep 15, 2022 • edited Loading

Description

mmarchetti left a comment

Choose a reason for hiding this comment

bcwu commented Sep 15, 2022 • edited Loading

kgartland-rstudio commented Sep 19, 2022

kgartland-rstudio commented Sep 19, 2022 • edited Loading

kgartland-rstudio commented Sep 19, 2022 • edited Loading

bcwu commented Sep 19, 2022

kgartland-rstudio commented Sep 19, 2022

bcwu commented Sep 19, 2022

kgartland-rstudio commented Sep 19, 2022

kgartland-rstudio commented Sep 19, 2022

bcwu commented Sep 19, 2022

kgartland-rstudio commented Sep 19, 2022

bcwu commented Sep 19, 2022

kgartland-rstudio commented Sep 20, 2022

kgartland-rstudio commented Sep 21, 2022

bcwu commented Sep 27, 2022 • edited Loading

kgartland-rstudio commented Sep 29, 2022

bcwu commented Sep 29, 2022

kgartland-rstudio commented Sep 29, 2022

bcwu commented Sep 29, 2022

bcwu commented Sep 15, 2022 •

edited

Loading

bcwu commented Sep 15, 2022 •

edited

Loading

kgartland-rstudio commented Sep 19, 2022 •

edited

Loading

kgartland-rstudio commented Sep 19, 2022 •

edited

Loading

bcwu commented Sep 27, 2022 •

edited

Loading