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.

Sign up for GitHub

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

Jump to bottom

Update workflows to use Python 3.7 for ensembler engines and sdk #190

Merged
merged 4 commits into from
Mar 30, 2022
Merged

Update workflows to use Python 3.7 for ensembler engines and sdk #190

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c7cbf03
Update workflows to use Python 3.7 for ensembler engines and sdk
deadlycoconuts Mar 29, 2022
bc44146
Add none return option for config in Router SDK class
deadlycoconuts Mar 29, 2022
0dae774
Update text display settings to display entire image name
deadlycoconuts Mar 29, 2022
02c5884
Set container image name to overflow instead of being truncated
deadlycoconuts Mar 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/pyfunc-ensembler-job.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
- name: Setup Python
uses: actions/setup-python@v2
with:
python-version: 3.8
python-version: 3.7

- name: Setup Java
uses: actions/setup-java@v2
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pyfunc-ensembler-service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
- name: Setup Python
uses: actions/setup-python@v2
with:
python-version: 3.8
python-version: 3.7

- name: Setup Conda
uses: conda-incubator/setup-miniconda@v2
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/sdk.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
- name: Setup Python
uses: actions/setup-python@v2
with:
python-version: 3.8
python-version: 3.7

- name: Cache pip dependencies
uses: actions/cache@v2
Expand Down Expand Up @@ -84,7 +84,7 @@ jobs:
- name: Setup Python
uses: actions/setup-python@v2
with:
python-version: 3.8
python-version: 3.7

- name: Cache pip dependencies
uses: actions/cache@v2
Expand Down
9 changes: 6 additions & 3 deletions sdk/turing/router/router.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import time
import logging

from typing import List, Dict
from typing import List, Dict, Optional

import turing.generated.models
from turing._base_types import ApiObject, ApiObjectSpec
Expand Down Expand Up @@ -75,8 +75,11 @@ def status(self) -> RouterStatus:
return self._status

@property
def config(self) -> 'RouterConfig':
return RouterConfig(name=self.name, environment_name=self.environment_name, **self._config)
def config(self) -> Optional['RouterConfig']:
if self._config is not None:
return RouterConfig(name=self.name, environment_name=self.environment_name, **self._config)
else:
return None
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this if-else check to handle cases whereby querying the API for a certain router returns an object without a RouterConfig. This occurs notably when a new Turing Router is pending (being deployed).


@property
def version(self) -> int:
Expand Down
2 changes: 1 addition & 1 deletion ui/src/components/config_section/index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
.euiPanel--configSection {
.euiDescriptionList.euiDescriptionList--responsiveColumn {
> * {
@include euiTextTruncate;
@include euiTextBreakWord;
Copy link
Contributor Author

@deadlycoconuts deadlycoconuts Mar 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@krithika369, I can't seem to override this mixin that truncates quite a number of text descriptions in other UI components - I tried creating custom style rules for the Container table but they don't seem to be able override this mixin either :/ Would it be too drastic if I were to swap this mixin out? I'm not sure why the decision was made to truncate long text in other config display panels though.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh we shouldn't change it globally for all rows.

I tried creating custom style rules for the Container table

Could you share the example here? What was tried and where it was placed?

Copy link
Contributor Author

@deadlycoconuts deadlycoconuts Mar 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I tried creating this custom style class and using it for the EuiDecriptionList of a ContainerConfigTable, but it doesn't overwrite any of the truncation styles:

.euiDescriptionList--responsiveColumn.containerConfig {
  .euiDescriptionList__description {
    @include euiTextBreakWord;
  }
}

Explicitly specifying the style of the euiTextBreakWord mixin as found here doesn't seem to work either:

.euiDescriptionList--responsiveColumn.containerConfig {
  .euiDescriptionList__description {
    overflow-wrap: break-word !important;
    word-wrap: break-word !important;
    word-break: break-word;
  }
}

Copy link
Contributor Author

@deadlycoconuts deadlycoconuts Mar 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thing I've tried to try was to add <div className="eui-textBreakWord"> tags to the description of the item corresponding to the image name within the items list of the ContainerConfigTable as:

{
  title: "Image",
  description: (
      <div className="eui-textBreakWord">
        thisisaverylargexcepetionnamewithalotofwordsthatatsomepointaregoingtobetruncated
      </div>
    )
}

Although this doesn't 'truncate' the line and replace the missing characters with an ellipsis, it doesn't break the line either... generating something like this instead:
Screenshot 2022-03-30 at 10 59 51 AM

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.. I was going to suggest something along the lines of your second example to override the properties for the single row's description. But adding the div is probably messing with some other flex properties. I will try to have a stab at it to understand better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! I'll see what I can try out from my end!

Copy link
Collaborator

@krithika369 krithika369 Mar 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using span in combination with inline style overrides seems to do it:

const items = [
    {
      title: "Image",
      description: (<span style={{
        wordBreak: "break-word",
        textOverflow: "unset",
        whiteSpace: "break-spaces"
      }} >
        {image}
      </span >),
    },

We're having to override some existing !important properties which doesn't seem easy with classes.
Screenshot 2022-03-30 at 1 28 45 PM

It may not be the most graceful solution but given that we want to relax the truncation behavior for the one description only, I think it may be acceptable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow thanks for discovering that! I'll include your code snippet in!


line-height: 2.1 !important;
margin-top: 2px !important;
Expand Down