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.

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

Update Scripts With FC REST Updates #32

Merged
merged 9 commits into from
Oct 4, 2021

Conversation

ric-evans
Copy link
Member

closes #31

@ric-evans ric-evans added bug Something isn't working enhancement New feature or request labels Sep 30, 2021
@ric-evans ric-evans self-assigned this Sep 30, 2021
Copy link
Contributor

@blinkdog blinkdog left a comment

Choose a reason for hiding this comment

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

LGTM.

Out of scope for this PR, but I wonder since we're (re)using the logical_name + query construct, if we should add a query shortcut inside the FC to mean this?

@ric-evans
Copy link
Member Author

ric-evans commented Oct 4, 2021

LGTM.

Out of scope for this PR, but I wonder since we're (re)using the logical_name + query construct, if we should add a query shortcut inside the FC to mean this?

It's a good idea since we're making it a construct here. But I'm hesitant to introduce more abstraction to the query interface considering we've departed the "logical_name=locations.path" paradigm. Really, this 2-arg construct is a stop-gap measure since locations.path is not currently suitably indexed to be the sole argument. I do see the benefit of introducing a location-path shortcut key (or similar) that bypasses the "query" argument.

@ric-evans
Copy link
Member Author

LGTM.
Out of scope for this PR, but I wonder since we're (re)using the logical_name + query construct, if we should add a query shortcut inside the FC to mean this?

It's a good idea since we're making it a construct here. But I'm hesitant to introduce more abstraction to the query interface considering we've departed the "logical_name=locations.path" paradigm. Really, this 2-arg construct is a stop-gap measure since locations.path is not currently suitably indexed to be the sole argument. I do see the benefit of introducing a location-path shortcut key (or similar) that bypasses the "query" argument.

Note: I'm not sure if we'll ever get this version of the FC to index locations.path well. It's something that Clowder may be able to do.

@ric-evans ric-evans merged commit 0ca93e0 into master Oct 4, 2021
@ric-evans ric-evans deleted the fc-query-logical-name-updates branch October 4, 2021 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Scripts With FC REST Updates
2 participants