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

feat: Flatten providers into strings per role TDE-1291 #1108

Merged
merged 6 commits into from
Oct 21, 2024

Conversation

l0b0
Copy link
Contributor

@l0b0 l0b0 commented Oct 14, 2024

Motivation

Make this information easier to process for GIS tools.

Checklist

  • Tests updated
  • Docs updated
  • Issue linked in Title

@l0b0 l0b0 requested review from a team as code owners October 14, 2024 22:25
@paulfouquet
Copy link
Collaborator

NIT: we don't have a convention in place in this repo for specifying the domain of the commit, example feat(GeoJSON)

@l0b0
Copy link
Contributor Author

l0b0 commented Oct 15, 2024

NIT: we don't have a convention in place in this repo for specifying the domain of the commit, example feat(GeoJSON)

Not sure whether we need a convention; isn't the domain meant to be more of a free-form hint about what the subject is?

@blacha
Copy link
Member

blacha commented Oct 15, 2024

from the conventional commits spec:

A scope MAY be provided after a type. A scope MUST consist of a noun describing a section of the codebase surrounded by parenthesis, e.g., fix(parser)

Im not sure we use many scopes in this repo, In the commit history the command name is sometimes used as the scope eg feat(copy)

I am not sure we have a "geojson" section of the codebase so IMO this should be either no scope or related to the command "mapsheet-coverage" ?

It would be nice to document the common scopes if we do want to be consistent the linter can limit which scopes we use.

@l0b0 l0b0 changed the title feat(GeoJSON): Flatten providers into strings per role TDE-1291 feat: Flatten providers into strings per role TDE-1291 Oct 15, 2024
@paulfouquet paulfouquet added the container Build a container from a pull request label Oct 15, 2024
@l0b0 l0b0 requested a review from blacha October 16, 2024 20:32
Wentao-Kuang and others added 4 commits October 17, 2024 13:34
…-1088 (#1109)

#### Motivation

We are now importing elevation data from `nz-elevation` bucket. So, it
should be

#### Modification

Add `nz-elevation` as valid source bucket.

#### Checklist

_If not applicable, provide explanation of why._

- [ ] Tests updated
- [ ] Docs updated
- [x] Issue linked in Title
…tly TDE-1268 (#1092)

#### Motivation

file systems are only created for unique roleArns so FileSystemCreated
event is not fired twice if two configuration objects have the same
roleArn,

give a confiugration with two buckets both using `roleA` on a service
that has a default role `roleDefault`
```
s3://foo/ - roleA
s3://bar/ - roleA
```

requests to 

```typescript
fs.read("s3://foo"); // tries roleDefault then uses roleA
fs.read("s3://foo"); // uses cached roleA
```

depending on the order of reads the default role may be used far too
often

```typescript
fs.read("s3://foo") // tries roleDefault then uses roleA
fs.read("s3://bar") // tries roleDefault then uses roleA
fs.read("s3://bar") // tries roleDefault then uses roleA
```

after this change

```typescript
fs.read("s3://foo") // tries roleDefault then uses roleA
fs.read("s3://bar") // tries roleDefault then uses roleA
fs.read("s3://bar") // uses cached roleA
```


#### Modification

hook the file system finder when it needs to find a new file system use
that file system to register onto `fsa`

#### Checklist

_If not applicable, provide explanation of why._

- [ ] Tests updated
- [ ] Docs updated
- [ ] Issue linked in Title
@l0b0 l0b0 requested a review from dwsilk October 20, 2024 21:44
@l0b0 l0b0 enabled auto-merge October 21, 2024 20:31
@l0b0 l0b0 added this pull request to the merge queue Oct 21, 2024
Merged via the queue into master with commit 92af2f9 Oct 21, 2024
2 checks passed
@l0b0 l0b0 deleted the feat/flatten-stac-providers-in-geojson branch October 21, 2024 20:33
@github-actions github-actions bot mentioned this pull request Oct 20, 2024
github-merge-queue bot pushed a commit that referenced this pull request Oct 21, 2024
🤖 I have created a release *beep* *boop*
---


## [4.7.0](v4.6.0...v4.7.0)
(2024-10-21)


### Features

* **basemaps:** Add nz-elevation bucket as valid source s3 bucket.
BM-1088 ([#1109](#1109))
([bc09b74](bc09b74))
* expose method to calculate a sheet code from any x,y
([#1110](#1110))
([ffa03ad](ffa03ad))
* Flatten providers into strings per role TDE-1291
([#1108](#1108))
([92af2f9](92af2f9))


### Bug Fixes

* ensure file systems with matching roleArns are registered correctly
TDE-1268 ([#1092](#1092))
([e004506](e004506))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
container Build a container from a pull request
Development

Successfully merging this pull request may close these issues.

5 participants