Skip to content

Fix the missing rest engine context#976

Merged
LanutiEmanuele merged 6 commits into
developfrom
fix/rest-engine-context
May 7, 2025
Merged

Fix the missing rest engine context#976
LanutiEmanuele merged 6 commits into
developfrom
fix/rest-engine-context

Conversation

@LanutiEmanuele

Copy link
Copy Markdown
Contributor

Fixed how set engine='rest' updates rest parameters

Description

When the cli was started with pandas and engine='rest' was set, the loaded data remained the default.

I created a 'set_rest_engine' function in shared/utils to extrapolate the correct data to update the parameters of the rest engine.
I updated the rest params in context_commands.set_ctxt.
I updated SQContext's post_init method

Type of change

  • Bug fix (non-breaking change which fixes an issue)

  • I have read the comments and followed the CONTRIBUTING.md.

  • I have explained my PR according to the information in the comments or in a linked issue.

  • My PR source branch is created from the develop branch.

  • My PR targets the develop branch.

  • All my commits have --signoff applied

Comment thread suzieq/shared/utils.py Outdated
Comment thread tests/unit/test_sqconfig_load.py Outdated
@LanutiEmanuele LanutiEmanuele force-pushed the fix/rest-engine-context branch from 4d871ae to 251f0bc Compare February 25, 2025 11:57

@rickstardust rickstardust left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One new mypy failure to fix

Comment thread tests/unit/test_sqconfig_load.py Outdated
@LanutiEmanuele LanutiEmanuele force-pushed the fix/rest-engine-context branch 2 times, most recently from a8ce6c4 to 3ccc995 Compare May 6, 2025 14:35

@Sepehr-A Sepehr-A left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving with a comment.

Comment thread suzieq/shared/utils.py Outdated

@ddutt ddutt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved with "please fix Sepehr's comment"

Signed-off-by: Lanuti_emanuele <emanuele.lanuti@stardustsystems.net>
Signed-off-by: Lanuti_emanuele <emanuele.lanuti@stardustsystems.net>
Signed-off-by: Lanuti_emanuele <emanuele.lanuti@stardustsystems.net>
Signed-off-by: Lanuti_emanuele <emanuele.lanuti@stardustsystems.net>
Signed-off-by: Emanuele Lanuti <emanuele.lanuti@stardustsystems.net>
Signed-off-by: Emanuele Lanuti <emanuele.lanuti@stardustsystems.net>
@LanutiEmanuele LanutiEmanuele force-pushed the fix/rest-engine-context branch from 3ccc995 to bf843ca Compare May 7, 2025 13:11
@LanutiEmanuele LanutiEmanuele merged commit 8ab720e into develop May 7, 2025
11 checks passed
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.

5 participants