-
Notifications
You must be signed in to change notification settings - Fork 53
Presto plugin executor #69
Changes from all commits
e5b6ccb
c0f71da
90478fd
0211489
526c3ac
ebcf04a
39a3a3c
2379221
c87dbd5
7ba7cc7
4baecf2
cd07b27
574a3cc
352ac58
f48673f
4dff686
2898c2c
23b3eb2
a3810ac
8a1954a
859d50f
177e7f9
76d0e96
7a5f2d6
7325020
78ac9ff
e639f60
3e87ce4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
package client | ||
|
||
import ( | ||
"context" | ||
"net/http" | ||
"net/url" | ||
|
||
"time" | ||
|
||
"github.com/lyft/flyteplugins/go/tasks/plugins/presto/config" | ||
) | ||
|
||
const ( | ||
httpRequestTimeoutSecs = 30 | ||
) | ||
|
||
type noopPrestoClient struct { | ||
client *http.Client | ||
environment *url.URL | ||
} | ||
|
||
func (p noopPrestoClient) ExecuteCommand( | ||
ctx context.Context, | ||
queryStr string, | ||
executeArgs PrestoExecuteArgs) (PrestoExecuteResponse, error) { | ||
|
||
return PrestoExecuteResponse{}, nil | ||
} | ||
|
||
func (p noopPrestoClient) KillCommand(ctx context.Context, commandID string) error { | ||
return nil | ||
} | ||
|
||
func (p noopPrestoClient) GetCommandStatus(ctx context.Context, commandID string) (PrestoStatus, error) { | ||
return NewPrestoStatus(ctx, "UNKNOWN"), nil | ||
} | ||
|
||
func NewNoopPrestoClient(cfg *config.Config) PrestoClient { | ||
return &noopPrestoClient{ | ||
client: &http.Client{Timeout: httpRequestTimeoutSecs * time.Second}, | ||
environment: cfg.Environment.ResolveReference(&cfg.Environment.URL), | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
package client | ||
EngHabu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
import "context" | ||
|
||
type PrestoStatus string | ||
|
||
// Contains information needed to execute a Presto query | ||
type PrestoExecuteArgs struct { | ||
RoutingGroup string `json:"routingGroup,omitempty"` | ||
Catalog string `json:"catalog,omitempty"` | ||
Schema string `json:"schema,omitempty"` | ||
Source string `json:"source,omitempty"` | ||
User string `json:"user,omitempty"` | ||
} | ||
|
||
// Representation of a response after submitting a query to Presto | ||
type PrestoExecuteResponse struct { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: add json tags |
||
ID string `json:"id,omitempty"` | ||
Status PrestoStatus `json:"status,omitempty"` | ||
NextURI string `json:"nextUri,omitempty"` | ||
} | ||
|
||
//go:generate mockery -all -case=snake | ||
|
||
// Interface to interact with PrestoClient for Presto tasks | ||
type PrestoClient interface { | ||
// Submits a query to Presto | ||
ExecuteCommand(ctx context.Context, commandStr string, executeArgs PrestoExecuteArgs) (PrestoExecuteResponse, error) | ||
|
||
// Cancels a currently running Presto query | ||
KillCommand(ctx context.Context, commandID string) error | ||
|
||
// Gets the status of a Presto query | ||
GetCommandStatus(ctx context.Context, commandID string) (PrestoStatus, error) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
package client | ||
|
||
import ( | ||
"context" | ||
"strings" | ||
|
||
"github.com/lyft/flytestdlib/logger" | ||
) | ||
|
||
// This type is meant only to encapsulate the response coming from Presto as a type, it is | ||
// not meant to be stored locally. | ||
const ( | ||
PrestoStatusUnknown PrestoStatus = "UNKNOWN" | ||
PrestoStatusWaiting PrestoStatus = "WAITING" | ||
PrestoStatusRunning PrestoStatus = "RUNNING" | ||
PrestoStatusFinished PrestoStatus = "FINISHED" | ||
PrestoStatusFailed PrestoStatus = "FAILED" | ||
PrestoStatusCancelled PrestoStatus = "CANCELLED" | ||
) | ||
|
||
var PrestoStatuses = map[PrestoStatus]struct{}{ | ||
EngHabu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
PrestoStatusUnknown: {}, | ||
PrestoStatusWaiting: {}, | ||
PrestoStatusRunning: {}, | ||
PrestoStatusFinished: {}, | ||
PrestoStatusFailed: {}, | ||
PrestoStatusCancelled: {}, | ||
} | ||
|
||
func NewPrestoStatus(ctx context.Context, state string) PrestoStatus { | ||
upperCased := strings.ToUpper(state) | ||
|
||
// Presto has different failure modes so this maps them all to a single Failure on the | ||
// Flyte side | ||
if strings.Contains(upperCased, "FAILED") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is failed singled out? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not entirely sure, I sort of copied this from the existing qubole_status. Probably not necessary to single this out. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, I think it was needed because Hive/Qubole has different error/failure modes and I think this maps them all to a single Failure/Error on the Flyte side. Going to leave this in for now There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add this as a comment? |
||
return PrestoStatusFailed | ||
} else if _, ok := PrestoStatuses[PrestoStatus(upperCased)]; ok { | ||
return PrestoStatus(upperCased) | ||
} else { | ||
logger.Warnf(ctx, "Invalid Presto Status found: %v", state) | ||
return PrestoStatusUnknown | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,76 @@ | ||||||
package config | ||||||
|
||||||
//go:generate pflags Config --default-var=defaultConfig | ||||||
|
||||||
import ( | ||||||
"context" | ||||||
"net/url" | ||||||
"time" | ||||||
|
||||||
"github.com/lyft/flytestdlib/config" | ||||||
"github.com/lyft/flytestdlib/logger" | ||||||
|
||||||
pluginsConfig "github.com/lyft/flyteplugins/go/tasks/config" | ||||||
) | ||||||
|
||||||
const prestoConfigSectionKey = "presto" | ||||||
|
||||||
func URLMustParse(s string) config.URL { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lower case?
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made it public because I call the method in one of the unit test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like URL. |
||||||
r, err := url.Parse(s) | ||||||
if err != nil { | ||||||
logger.Panicf(context.TODO(), "Bad Presto URL Specified as default, error: %s", err) | ||||||
} | ||||||
if r == nil { | ||||||
logger.Panicf(context.TODO(), "Nil Presto URL specified.", err) | ||||||
} | ||||||
return config.URL{URL: *r} | ||||||
} | ||||||
|
||||||
type RoutingGroupConfig struct { | ||||||
Name string `json:"name" pflag:",The name of a given Presto routing group"` | ||||||
Limit int `json:"limit" pflag:",Resource quota (in the number of outstanding requests) of the routing group"` | ||||||
ProjectScopeQuotaProportionCap float64 `json:"projectScopeQuotaProportionCap" pflag:",A floating point number between 0 and 1, specifying the maximum proportion of quotas allowed to allocate to a project in the routing group"` | ||||||
NamespaceScopeQuotaProportionCap float64 `json:"namespaceScopeQuotaProportionCap" pflag:",A floating point number between 0 and 1, specifying the maximum proportion of quotas allowed to allocate to a namespace in the routing group"` | ||||||
} | ||||||
|
||||||
type RefreshCacheConfig struct { | ||||||
Name string `json:"name" pflag:",The name of the rate limiter"` | ||||||
SyncPeriod config.Duration `json:"syncPeriod" pflag:",The duration to wait before the cache is refreshed again"` | ||||||
Workers int `json:"workers" pflag:",Number of parallel workers to refresh the cache"` | ||||||
LruCacheSize int `json:"lruCacheSize" pflag:",Size of the cache"` | ||||||
} | ||||||
|
||||||
var ( | ||||||
defaultConfig = Config{ | ||||||
Environment: URLMustParse(""), | ||||||
DefaultRoutingGroup: "adhoc", | ||||||
DefaultUser: "flyte-default-user", | ||||||
RoutingGroupConfigs: []RoutingGroupConfig{{Name: "adhoc", Limit: 250}, {Name: "etl", Limit: 100}}, | ||||||
RefreshCacheConfig: RefreshCacheConfig{ | ||||||
Name: "presto", | ||||||
SyncPeriod: config.Duration{Duration: 5 * time.Second}, | ||||||
Workers: 15, | ||||||
LruCacheSize: 10000, | ||||||
}, | ||||||
} | ||||||
|
||||||
prestoConfigSection = pluginsConfig.MustRegisterSubSection(prestoConfigSectionKey, &defaultConfig) | ||||||
) | ||||||
|
||||||
// Presto plugin configs | ||||||
type Config struct { | ||||||
Environment config.URL `json:"environment" pflag:",Environment endpoint for Presto to use"` | ||||||
DefaultRoutingGroup string `json:"defaultRoutingGroup" pflag:",Default Presto routing group"` | ||||||
DefaultUser string `json:"defaultUser" pflag:",Default Presto user"` | ||||||
RoutingGroupConfigs []RoutingGroupConfig `json:"routingGroupConfigs" pflag:"-,A list of cluster configs. Each of the configs corresponds to a service cluster"` | ||||||
RefreshCacheConfig RefreshCacheConfig `json:"refreshCacheConfig" pflag:"Rate limiter config"` | ||||||
} | ||||||
|
||||||
// Retrieves the current config value or default. | ||||||
func GetPrestoConfig() *Config { | ||||||
return prestoConfigSection.GetConfig().(*Config) | ||||||
} | ||||||
|
||||||
func SetPrestoConfig(cfg *Config) error { | ||||||
return prestoConfigSection.SetConfig(cfg) | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a noop Presto client for the open-source version. There will be a separate commit for one based on Mozart