Skip to content

Add function and script that return IDs of open positions#289

Merged
illia-malachyn merged 5 commits intov0from
illia-malachyn/add-get-position-ids-script
Mar 26, 2026
Merged

Add function and script that return IDs of open positions#289
illia-malachyn merged 5 commits intov0from
illia-malachyn/add-get-position-ids-script

Conversation

@illia-malachyn
Copy link
Collaborator

@illia-malachyn illia-malachyn commented Mar 24, 2026

Adding a script to get the IDs of open positions. FCM observer needs this information to query the position health factors.

Update: backported to main #296

@illia-malachyn illia-malachyn requested a review from a team as a code owner March 24, 2026 12:44
@illia-malachyn illia-malachyn force-pushed the illia-malachyn/add-get-position-ids-script branch from 17e1ed9 to b929229 Compare March 24, 2026 12:45
@illia-malachyn illia-malachyn force-pushed the illia-malachyn/add-get-position-ids-script branch from b929229 to 65bd300 Compare March 24, 2026 13:38
@illia-malachyn
Copy link
Collaborator Author

illia-malachyn commented Mar 25, 2026

I've just understood that it might be better to write a script that fetches a number of positions with an offset. Sth like:

access(all) fun main(startIndex: UInt64, count: UInt64): [FlowALPv0.PositionDetails];

This is doable and it's a faster way to retrieve positions. I'll update PR with a new script

@illia-malachyn illia-malachyn marked this pull request as draft March 25, 2026 13:43
@illia-malachyn illia-malachyn marked this pull request as ready for review March 25, 2026 14:48
@illia-malachyn
Copy link
Collaborator Author

illia-malachyn commented Mar 25, 2026

I think with such a script it will be easier to manage positions #294

@nialexsan
Copy link
Collaborator

@illia-malachyn can you also create a similar PR to the main branch and link it here to keep track of backported features

Copy link
Member

@jordanschalm jordanschalm left a comment

Choose a reason for hiding this comment

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

I prefer this solution over #294. I feel like dealing with both position ID and index is a bit confusing.

If we pair this with a script to batch-retrieve position details by list of position IDs (to avoid needing one script call per position), I think we'll have what we need.

@UlyanaAndrukhiv UlyanaAndrukhiv self-requested a review March 25, 2026 16:42
Copy link
Contributor

@UlyanaAndrukhiv UlyanaAndrukhiv left a comment

Choose a reason for hiding this comment

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

I like the current approach, think it is better cause returns all position ids in a single call.

@illia-malachyn
Copy link
Collaborator Author

If we pair this with a script to batch-retrieve position details by list of position IDs (to avoid needing one script call per position), I think we'll have what we need.

Exactly! That is what I was thinking of as the 3rd approach. I'll add such a script to this PR

@illia-malachyn illia-malachyn merged commit 1eb19fc into v0 Mar 26, 2026
1 check passed
@illia-malachyn illia-malachyn deleted the illia-malachyn/add-get-position-ids-script branch March 26, 2026 10:26
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