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

Raw output prefix takes precedence for remote proxy creation in entrypoint #572

Merged
merged 5 commits into from
Jul 30, 2021

Conversation

wild-endeavor
Copy link
Contributor

@wild-endeavor wild-endeavor commented Jul 30, 2021

TL;DR

When running in GCP without a config file present, flytekit uses the wrong setting to try to determine what environment it's running in. We should base the decision on the offloaded data location, not the cloud provider setting, which defaults to AWS.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

Add an extra conditional to first check the raw_output_data_prefix. Not too worried about the increased complexity becase this section of the code will soon be removed by #559, which should be going in within a week and change.

@wild-endeavor wild-endeavor changed the title Don't query for cloud provider on entrypoint Raw output prefix takes precedence for remote proxy creation in entrypoint Jul 30, 2021
@codecov
Copy link

codecov bot commented Jul 30, 2021

Codecov Report

Merging #572 (fc970fa) into master (716ccc7) will not change coverage.
The diff coverage is n/a.

❗ Current head fc970fa differs from pull request most recent head 96431e3. Consider uploading reports for the commit 96431e3 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #572   +/-   ##
=======================================
  Coverage   85.47%   85.47%           
=======================================
  Files         376      376           
  Lines       29265    29265           
  Branches     2350     2350           
=======================================
  Hits        25014    25014           
  Misses       3616     3616           
  Partials      635      635           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 716ccc7...96431e3. Read the comment docs.

@wild-endeavor wild-endeavor marked this pull request as ready for review July 30, 2021 01:43
Signed-off-by: wild-endeavor <[email protected]>
Signed-off-by: wild-endeavor <[email protected]>
Signed-off-by: wild-endeavor <[email protected]>
Signed-off-by: wild-endeavor <[email protected]>
Signed-off-by: wild-endeavor <[email protected]>
@wild-endeavor wild-endeavor force-pushed the config-cloud-provider branch from fc970fa to 96431e3 Compare July 30, 2021 01:46
@wild-endeavor wild-endeavor merged commit de18a60 into master Jul 30, 2021
@wild-endeavor wild-endeavor mentioned this pull request Jul 30, 2021
kumare3 pushed a commit that referenced this pull request Jul 31, 2021
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.

3 participants