Skip to content

uplink api semantics#2001

Merged
Geal merged 6 commits intodevfrom
geal/uplink-api-semantics
Oct 27, 2022
Merged

uplink api semantics#2001
Geal merged 6 commits intodevfrom
geal/uplink-api-semantics

Conversation

@Geal
Copy link
Contributor

@Geal Geal commented Oct 25, 2022

Fix #1494
Fix #1539

The Uplink response must drive the behaviour of the schema downloader. Some errors indicate we should not retry.

@github-actions

This comment has been minimized.

@Geal Geal requested review from abernix and o0Ignition0o October 26, 2022 08:18
@Geal
Copy link
Contributor Author

Geal commented Oct 26, 2022

@abernix how should we handle differences between the poll interval option passed as command line argument, VS the interval returned by Uplink? Should we use the Uplink one, but then override with the CLI arg if it is passed (and so, not set a default value for that one)

@Geal Geal self-assigned this Oct 26, 2022
@Geal Geal marked this pull request as ready for review October 26, 2022 12:06
@Geal Geal changed the title Geal/uplink api semantics uplink api semantics Oct 27, 2022
@abernix
Copy link
Member

abernix commented Oct 27, 2022

Hmm, I knew we had environment variables, but tbh I'd missed we had the ability to set this on the command line. That is unexpected to me and I'm not sure what the user story was for that since I'd say even environment variable usage would be relatively low.

Anyhow, to answer your question, I'd say the value provided by Uplink should always take precedence over anything set by the user since it's a backoff mechanism that our infrastructure teams can turn on if we need to slow things down. (Also we should make sure we have the minimum 10 second guardrail in place for user-configured values.)

(Frankly, I'd deprecate the CLI flag, but that doesn't have to happen here.)

@Geal Geal enabled auto-merge (squash) October 27, 2022 14:06
@Geal
Copy link
Contributor Author

Geal commented Oct 27, 2022

ok, I'm opening #2016 as a follow up

@Geal Geal merged commit af50f9a into dev Oct 27, 2022
@Geal Geal deleted the geal/uplink-api-semantics branch October 27, 2022 14:22
@abernix abernix mentioned this pull request Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

uplink: respect the minDelaySeconds field returned from Uplink tui polish: improve formatting of Uplink message on ACCESS_DENIED

4 participants