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

fix PATH_PREFIX handling in metadata service so it doesn't interfere with mfgui routes #388

Merged
merged 1 commit into from
Sep 7, 2023
Merged

fix PATH_PREFIX handling in metadata service so it doesn't interfere with mfgui routes #388

merged 1 commit into from
Sep 7, 2023

Conversation

oavdeev
Copy link
Collaborator

@oavdeev oavdeev commented Sep 5, 2023

follow up fix for #384 , without this it can break MFGUI

saikonen
saikonen reviewed Sep 6, 2023
View reviewed changes
services/metadata_service/server.py
@@ -34,16 +34,16 @@ def app(loop=None, db_conf: DBConfiguration = None, middlewares=None):
ArtificatsApi(app)
AuthApi(app)

if len(PATH_PREFIX) > 0:
_app.add_subapp(PATH_PREFIX, app)
if path_prefix:
Copy link
Collaborator

Choose a reason for hiding this comment

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

would prefer explicitly checking for the default if path_prefix is not None

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm I didn't want to treat "None" vs "" differently in any way here. Let me just change the function argument default to "" for symmetry

services/metadata_service/server.py
@@ -18,12 +18,12 @@
PATH_PREFIX = os.environ.get("PATH_PREFIX", "")
Copy link
Collaborator

Choose a reason for hiding this comment

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

change default to None instead of empty string to match the default value for prefix in app() ?

@saikonen
Copy link
Collaborator

saikonen commented Sep 6, 2023

for context why the existing changes would break MFGUI when using PATH_PREFIX: The ui_backend imports metadata serviceapp() and hosts it as a subapp. Therefore the setting of the prefix config needs to be kept outside of the app function in the metadata service.

@oavdeev oavdeev force-pushed the fix-path-prefix-metadata-service branch from 1755402 to 65d1ba8 Compare September 7, 2023 15:42
@saikonen saikonen merged commit a1cd81a into master Sep 7, 2023
6 checks passed
saikonen added a commit that referenced this pull request Oct 25, 2023
@saikonen @tfurmston
@romain-intel @oavdeev @RikishK
Patch pjoshi aurora (#395)
158e27f 
* Upgrade Github actions used in `dockerimage` action (#379)

* upgrade github actions used in dockerimage action

* remove setup-buildx-action and pin to hashes.

* change deprecated pkg_resources to importlib.metadata (#387)

* In a previous commit, the detection of a failure became too aggressive. (#386)

* In a previous commit, the detection of a failure became too aggressive.

This remediates this by considering a run 'failed' if the hb hasn't been
updated within heartbeat_cutoff time as opposed to the heartbeat_threshold time

* change run finished at query to heartbeat_cutoff from threshold

* clean up unused values from run query

---------

Co-authored-by: Sakari Ikonen <sakari.a.ikonen@gmail.com>

* fix PATH_PREFIX handling in metadata service so it doesn't interfere with mfgui routes (#388)

* Configurable SSL Connection (#373)

* [TRIS-297] Configurable SSL Connection (#1)

* Configurable SSL connection

* Update services/utils/__init__.py

* no ssl unit testing (#3)

* ssl seperate test (#4)

* dsn generator sslmode none (#5)

* fix run_goose.py not working without SSL mode env variables. (#390)

* change run inactive cutoff default to 6 minutes. cleanup unused constant (#392)

* clarify comment on read replica hosts

* make USE_SEPARATE_READER_POOL a boolean

* remove unnecessary conditionals for pool choice in execute_sql

---------

Co-authored-by: Tom Furmston <tfurmston@googlemail.com>
Co-authored-by: Romain <romain-intel@users.noreply.github.com>
Co-authored-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>
Co-authored-by: RikishK <69884402+RikishK@users.noreply.github.com>
saikonen added a commit that referenced this pull request Oct 30, 2023
@pjoshi30 @romain-intel
@wangchy27 @saikonen @tfurmston @oavdeev @RikishK
Added the ability to separate out reads and writes into their own con…
741b1a5 
…nection pools. (#344)

* Changes for using a separate reader pool for Aurora-like use cases

* Avoid some expensive logging operations when not needed

* Refactoring execute_sql implementations and separating reader/writer endpoints

choosing the right pool in execute_sql

* Adding documentation for using separate reader pools

* use [PREFIX]_READ_REPLICA_HOST as a feature gate instead of localhost

* In a previous commit, the detection of a failure became too aggressive.

This remediates this by considering a run 'failed' if the hb hasn't been
updated within heartbeat_cutoff time as opposed to the heartbeat_threshold time

* Patch pjoshi aurora (#395)

* Upgrade Github actions used in `dockerimage` action (#379)

* upgrade github actions used in dockerimage action

* remove setup-buildx-action and pin to hashes.

* change deprecated pkg_resources to importlib.metadata (#387)

* In a previous commit, the detection of a failure became too aggressive. (#386)

* In a previous commit, the detection of a failure became too aggressive.

This remediates this by considering a run 'failed' if the hb hasn't been
updated within heartbeat_cutoff time as opposed to the heartbeat_threshold time

* change run finished at query to heartbeat_cutoff from threshold

* clean up unused values from run query

---------

Co-authored-by: Sakari Ikonen <sakari.a.ikonen@gmail.com>

* fix PATH_PREFIX handling in metadata service so it doesn't interfere with mfgui routes (#388)

* Configurable SSL Connection (#373)

* [TRIS-297] Configurable SSL Connection (#1)

* Configurable SSL connection

* Update services/utils/__init__.py

* no ssl unit testing (#3)

* ssl seperate test (#4)

* dsn generator sslmode none (#5)

* fix run_goose.py not working without SSL mode env variables. (#390)

* change run inactive cutoff default to 6 minutes. cleanup unused constant (#392)

* clarify comment on read replica hosts

* make USE_SEPARATE_READER_POOL a boolean

* remove unnecessary conditionals for pool choice in execute_sql

---------

Co-authored-by: Tom Furmston <tfurmston@googlemail.com>
Co-authored-by: Romain <romain-intel@users.noreply.github.com>
Co-authored-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>
Co-authored-by: RikishK <69884402+RikishK@users.noreply.github.com>

* fix broken connection string after conflict resolve

* make codestyles happy

* fix test cases

* cleanup

* merge run_goose.py from master

* revert unnecessary changes

---------

Co-authored-by: Preetam Joshi <preetamj@netflix.com>
Co-authored-by: Romain Cledat <rcledat@netflix.com>
Co-authored-by: Chaoying Wang <chaoyingw@netflix.com>
Co-authored-by: Sakari Ikonen <64256562+saikonen@users.noreply.github.com>
Co-authored-by: Tom Furmston <tfurmston@googlemail.com>
Co-authored-by: Romain <romain-intel@users.noreply.github.com>
Co-authored-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>
Co-authored-by: RikishK <69884402+RikishK@users.noreply.github.com>
Co-authored-by: Sakari Ikonen <sakari.a.ikonen@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saikonen saikonen saikonen approved these changes

@savingoyal savingoyal Awaiting requested review from savingoyal

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@oavdeev @saikonen