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

Move basic project config from trails-viz to trails-viz-data #217

Open
jblehr opened this issue Sep 4, 2024 · 6 comments
Open

Move basic project config from trails-viz to trails-viz-data #217

jblehr opened this issue Sep 4, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@jblehr
Copy link
Contributor

jblehr commented Sep 4, 2024

Right now, a lot of the data updates to the dashboard involve minor updates to the trails-viz/trails-viz-api/trailsvizapi/config/app_config.py file. See here for the docs on this. For example, these are common updates to the dashboard:

  • Adding a new project requires adding the name of the new project and listing the data platforms to display, i.e., updating PROJECT_NAMES and DATA_COLUMNS.
  • Updating the platforms for an existing project requires updating DATA_COLUMNS.
  • Adding home locations for an existing project requires updating CENSUS_TRACT_STATES.

However, because we are changing the code within trails-viz-api, we need to build and push a new image. Instead, I'm proposing we move this code to within trails-viz-data, so that only actual changes to the code (and not just config) require new builds of the dashboard.

See below for an example of the dictionaries found within app_config.py:

# if the project code contains the project group keyword, then it belongs to that group
PROJECT_NAMES = {
    'West Cascades': 'WestCascades',
}

CENSUS_TRACT_STATES = {
    'WestCascades': ['53'],
}

DATA_COLUMNS = {
    'WestCascades': ['estimate', 'flickr', 'twitter', 'wta', 'alltrails', 'reveal', 'onsite'],
}

See recent #212 and #216 for examples of a PR/new build that I think should happen in trails-viz-data instead.

One reason (and a good one) I can see to continue including these in this repo is to allow for us to track different versions of the data that are shown on the dashboard in this repo (in addition to trails-viz-data).

@woodsp - what do you think about this proposal? Do you think this should be a component of upcoming dashboard development?

@jblehr jblehr added the enhancement New feature or request label Sep 4, 2024
@jblehr
Copy link
Contributor Author

jblehr commented Sep 19, 2024

Just noting here that @woodsp mentioned that we initially had good reasons for NOT doing this, but we couldn't remember what they were.

Since that is the case, we decided it would be worthwhile to try this out, since this would streamline dashboard updates significantly.

@EmiliaH
Copy link
Member

EmiliaH commented Dec 17, 2024

@jblehr and @davidye007 is this issue still relevant? I'm assuming it hasn't been closed by any of the existing PRs?

@davidye007
Copy link
Collaborator

@EmiliaH the CI/CD pipeline for the trails-viz repo will still be triggered if changes are made to app_config.py.

However, the updated CI/CD pipeline has streamlined the deployment process significantly. As a result, whether the app_config.py lives in the trails-viz repo or trails-viz-data repo, the amount of time/effort it would take to deploy these will be very similar.

Thus, if streamlining development (saving time/effort) is main goal, I think the updated CI/CD pipeline covers it and we don't need to move app_config.py over to trails-viz-data.

@jblehr Let me know if I'm missing something.

@jblehr
Copy link
Contributor Author

jblehr commented Dec 19, 2024

@davidye007, yes I agree that your (significant!) improvements to the deployment process has made this easier. However, I'm not sure it solves this issue.

I don't think the main goal is time savings during dashboard development (like what you're doing now).

I see the main goals as being time savings and reducing complexity during regular data updates to the dashboard. For example, when someone adds a new project to the dashboard, they already have to add a new folder and new data to the trails-viz-data folder (following docs here). However, then they also have to open a PR in this repo to change the app_config.py file. It'd be great if these could be bundled in one PR (in trails-viz-data). Again, let me know if you think this is a bad idea!

So I think we should leave this issue open, but I agree that your changes have made deployment easier and that this issue should be lower priority.

@EmiliaH
Copy link
Member

EmiliaH commented Dec 19, 2024

Thank you both for adding thoughts/context, that was helpful. Based on that, I think it makes sense to move app_config.py to trails-viz-data so that we can add a project in one fell PR swoop (decrease the number of steps that happen in different places).

So @davidye007, if you don't think there's any issues with this proposal, then I think we should implement this. It seems like a fairly easy fix.

@sgwinder, FYI.

@EmiliaH
Copy link
Member

EmiliaH commented Dec 26, 2024

related to issue #251

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants